From e650076e0f6cf3962f16d093e3446b5eb62eaf03 Mon Sep 17 00:00:00 2001 From: mprahl Date: Fri, 17 May 2024 11:33:06 -0400 Subject: [PATCH] Add support for recording the diff in the ConfigurationPolicy status A new recordDiff option of "InStatus" allows the diff to be stored in the object properties in the ConfigurationPolicy status. The new default recordDiff value is "InStatus" unless sensitive data may be in the diff. Then the user must explicitly set recordDiff. Relates: https://issues.redhat.com/browse/ACM-11421 Signed-off-by: mprahl --- api/v1/configurationpolicy_types.go | 44 ++++- api/v1/configurationpolicy_types_test.go | 86 +++++++++ controllers/configurationpolicy_controller.go | 170 +++++++++++++----- controllers/configurationpolicy_utils.go | 27 +-- controllers/configurationpolicy_utils_test.go | 6 +- ...r-management.io_configurationpolicies.yaml | 6 +- .../kustomization.yaml | 6 + ...luster-management.io_operatorpolicies.yaml | 2 + .../kustomize_operatorpolicy/remove-diff.json | 6 + ...r-management.io_configurationpolicies.yaml | 6 +- go.mod | 4 +- go.sum | 4 +- test/e2e/case20_delete_objects_test.go | 6 +- test/e2e/case2_role_handling_test.go | 33 +++- test/e2e/case39_diff_generation_test.go | 166 ++++++++++++++++- test/e2e/case8_status_check_test.go | 25 ++- .../case39-no-diff-object-templates-raw.yaml | 15 ++ .../case39-no-diff-object-templates.yaml | 15 ++ .../case39-no-diff-on-secret.yaml | 17 ++ .../case39_diff_generation/case39-secret.yaml | 8 + .../case39-truncated.yaml | 69 +++++++ 21 files changed, 645 insertions(+), 76 deletions(-) create mode 100644 api/v1/configurationpolicy_types_test.go create mode 100644 deploy/crds/kustomize_operatorpolicy/remove-diff.json create mode 100644 test/resources/case39_diff_generation/case39-no-diff-object-templates-raw.yaml create mode 100644 test/resources/case39_diff_generation/case39-no-diff-object-templates.yaml create mode 100644 test/resources/case39_diff_generation/case39-no-diff-on-secret.yaml create mode 100644 test/resources/case39_diff_generation/case39-secret.yaml create mode 100644 test/resources/case39_diff_generation/case39-truncated.yaml diff --git a/api/v1/configurationpolicy_types.go b/api/v1/configurationpolicy_types.go index 9411ab40..e0a07f67 100644 --- a/api/v1/configurationpolicy_types.go +++ b/api/v1/configurationpolicy_types.go @@ -11,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" runtime "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -199,16 +200,48 @@ type ObjectTemplate struct { ObjectDefinition runtime.RawExtension `json:"objectDefinition"` // RecordDiff specifies whether (and where) to log the diff between the object on the - // cluster and the objectDefinition in the policy. Defaults to "None". + // cluster and the objectDefinition in the policy. Defaults to "None" when the object kind is + // ConfigMap, OAuthAccessToken, OAuthAuthorizeTokens, Route, or Secret. Defaults to "InStatus" otherwise. RecordDiff RecordDiff `json:"recordDiff,omitempty"` } -// +kubebuilder:validation:Enum=Log;None +func (o *ObjectTemplate) RecordDiffWithDefault() RecordDiff { + if o.RecordDiff != "" { + return o.RecordDiff + } + + _, gvk, err := unstructured.UnstructuredJSONScheme.Decode(o.ObjectDefinition.Raw, nil, nil) + if err != nil { + return o.RecordDiff + } + + switch gvk.Group { + case "": + if gvk.Kind == "ConfigMap" || gvk.Kind == "Secret" { + return RecordDiffCensored + } + case "oauth.openshift.io": + if gvk.Kind == "OAuthAccessToken" || gvk.Kind == "OAuthAuthorizeTokens" { + return RecordDiffCensored + } + case "route.openshift.io": + if gvk.Kind == "Route" { + return RecordDiffCensored + } + } + + return RecordDiffInStatus +} + +// +kubebuilder:validation:Enum=Log;InStatus;None type RecordDiff string const ( - RecordDiffLog RecordDiff = "Log" - RecordDiffNone RecordDiff = "None" + RecordDiffLog RecordDiff = "Log" + RecordDiffInStatus RecordDiff = "InStatus" + RecordDiffNone RecordDiff = "None" + // Censored is only used as an internal value to indicate a diff shouldn't be automatically generated. + RecordDiffCensored RecordDiff = "Censored" ) // ConfigurationPolicyStatus defines the observed state of ConfigurationPolicy @@ -355,7 +388,8 @@ type ObjectProperties struct { // Whether the object was created by the parent policy CreatedByPolicy *bool `json:"createdByPolicy,omitempty"` // Store object UID to help track object ownership for deletion - UID string `json:"uid,omitempty"` + UID string `json:"uid,omitempty"` + Diff string `json:"diff,omitempty"` } func init() { diff --git a/api/v1/configurationpolicy_types_test.go b/api/v1/configurationpolicy_types_test.go new file mode 100644 index 00000000..edba5598 --- /dev/null +++ b/api/v1/configurationpolicy_types_test.go @@ -0,0 +1,86 @@ +// Copyright Contributors to the Open Cluster Management project + +package v1 + +import ( + "fmt" + "testing" + + "k8s.io/apimachinery/pkg/runtime" +) + +func TestRecordDiffWithDefault(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + apiVersion string + kind string + recordDiff RecordDiff + expected RecordDiff + }{ + "Namespace-recordDiff-unset": { + apiVersion: "v1", + kind: "Namespace", + expected: RecordDiffInStatus, + }, + "Namespace-recordDiff-set": { + apiVersion: "v1", + kind: "Namespace", + recordDiff: RecordDiffLog, + expected: RecordDiffLog, + }, + "Configmap-recordDiff-unset": { + apiVersion: "v1", + kind: "ConfigMap", + expected: RecordDiffCensored, + }, + "Secret-recordDiff-unset": { + apiVersion: "v1", + kind: "Secret", + expected: RecordDiffCensored, + }, + "Secret-recordDiff-set": { + apiVersion: "v1", + kind: "Secret", + recordDiff: RecordDiffInStatus, + expected: RecordDiffInStatus, + }, + "OAuthAccessToken-recordDiff-unset": { + apiVersion: "oauth.openshift.io/v1", + kind: "OAuthAccessToken", + expected: RecordDiffCensored, + }, + "OAuthAuthorizeTokens-recordDiff-unset": { + apiVersion: "oauth.openshift.io/v1", + kind: "OAuthAuthorizeTokens", + expected: RecordDiffCensored, + }, + "Route-recordDiff-unset": { + apiVersion: "route.openshift.io/v1", + kind: "Route", + expected: RecordDiffCensored, + }, + } + + for testName, test := range tests { + test := test + + t.Run( + testName, + func(t *testing.T) { + t.Parallel() + + objTemp := ObjectTemplate{ + ObjectDefinition: runtime.RawExtension{ + Raw: []byte(fmt.Sprintf(`{"apiVersion": "%s", "kind": "%s"}`, test.apiVersion, test.kind)), + }, + RecordDiff: test.recordDiff, + } + + if objTemp.RecordDiffWithDefault() != test.expected { + t.Fatalf("Expected %s but got %s", test.expected, objTemp.RecordDiffWithDefault()) + } + }, + ) + } +} diff --git a/controllers/configurationpolicy_controller.go b/controllers/configurationpolicy_controller.go index 7226b9bb..fdcb8fce 100644 --- a/controllers/configurationpolicy_controller.go +++ b/controllers/configurationpolicy_controller.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "github.com/go-logr/logr" gocmp "github.com/google/go-cmp/cmp" "github.com/prometheus/client_golang/prometheus" templates "github.com/stolostron/go-template-utils/v4/pkg/templates" @@ -1030,11 +1031,34 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi return } + if resolvedTemplate.HasSensitiveData { + for i := range objTemps { + if objTemps[i].RecordDiff == "" { + log.V(1).Info( + "Not automatically turning on recordDiff due to templates interacting with "+ + "sensitive data", + "objectTemplateIndex", i, + ) + + objTemps[i].RecordDiff = policyv1.RecordDiffCensored + } + } + } + plc.Spec.ObjectTemplates = objTemps break } + if plc.Spec.ObjectTemplates[i].RecordDiff == "" && resolvedTemplate.HasSensitiveData { + log.V(1).Info( + "Not automatically turning on recordDiff due to templates interacting with sensitive data", + "objectTemplateIndex", i, + ) + + plc.Spec.ObjectTemplates[i].RecordDiff = policyv1.RecordDiffCensored + } + // Otherwise, set the resolved data for use in further processing plc.Spec.ObjectTemplates[i].ObjectDefinition.Raw = resolvedTemplate.ResolvedJSON } else if isRawObjTemplate { @@ -1306,7 +1330,8 @@ func (r *ConfigurationPolicyReconciler) sortRelatedObjectsAndUpdate( newEntry.Properties.CreatedByPolicy != nil && !(*newEntry.Properties.CreatedByPolicy) { // Use the old properties if they existed and this is not a newly created resource - related[i].Properties = oldEntry.Properties + related[i].Properties.CreatedByPolicy = oldEntry.Properties.CreatedByPolicy + related[i].Properties.UID = oldEntry.Properties.UID if collectMetrics { found[objKey] = true @@ -1552,9 +1577,9 @@ func (r *ConfigurationPolicyReconciler) handleObjects( log.V(2).Info("Handling a single object template") - var creationInfo *policyv1.ObjectProperties + var objectProperties *policyv1.ObjectProperties - result, creationInfo = r.handleSingleObj(singObj, remediation, exists, objectT) + result, objectProperties = r.handleSingleObj(singObj, remediation, exists, objectT) if len(result.events) != 0 { event := result.events[len(result.events)-1] @@ -1566,7 +1591,7 @@ func (r *ConfigurationPolicyReconciler) handleObjects( objDetails.isNamespaced, result.objectNames, event.reason, - creationInfo, + objectProperties, ) } } else { // This case only occurs when the desired object is not named @@ -1668,7 +1693,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( objectT *policyv1.ObjectTemplate, ) ( result objectTmplEvalResult, - creationInfo *policyv1.ObjectProperties, + objectProperties *policyv1.ObjectProperties, ) { objLog := log.WithValues("object", obj.name, "policy", obj.policy.Name, "index", obj.index) @@ -1706,7 +1731,7 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( objLog.Error(err, "Could not handle missing musthave object") } else { created := true - creationInfo = &policyv1.ObjectProperties{ + objectProperties = &policyv1.ObjectProperties{ CreatedByPolicy: &created, UID: uid, } @@ -1745,14 +1770,25 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( log.V(2).Info("The object already exists. Verifying the object fields match what is desired.") var throwSpecViolation, triedUpdate, updatedObj bool - var msg string + var msg, diff string + + uid := string(obj.existingObj.GetUID()) if evaluated, compliant := r.alreadyEvaluated(obj.policy, obj.existingObj); evaluated { log.V(1).Info("Skipping object comparison since the resourceVersion hasn't changed") + for _, relatedObj := range obj.policy.Status.RelatedObjects { + if relatedObj.Properties != nil && relatedObj.Properties.UID == uid { + // Retain the diff from the previous evaluation + diff = relatedObj.Properties.Diff + + break + } + } + throwSpecViolation = !compliant } else { - throwSpecViolation, msg, triedUpdate, updatedObj = r.checkAndUpdateResource( + throwSpecViolation, msg, diff, triedUpdate, updatedObj = r.checkAndUpdateResource( obj, objectT, remediation, ) } @@ -1762,6 +1798,8 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( result.events = append(result.events, objectTmplEvalEvent{false, reasonWantFoundNoMatch, ""}) } + created := false + if throwSpecViolation { var resultReason, resultMsg string @@ -1772,6 +1810,12 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( resultReason = reasonWantFoundNoMatch } + objectProperties = &policyv1.ObjectProperties{ + CreatedByPolicy: &created, + UID: uid, + Diff: diff, + } + result.events = append(result.events, objectTmplEvalEvent{false, resultReason, resultMsg}) } else { // it is a must have and it does exist, so it is compliant @@ -1781,10 +1825,11 @@ func (r *ConfigurationPolicyReconciler) handleSingleObj( } else { result.events = append(result.events, objectTmplEvalEvent{true, reasonWantFoundExists, ""}) } - created := false - creationInfo = &policyv1.ObjectProperties{ + + objectProperties = &policyv1.ObjectProperties{ CreatedByPolicy: &created, - UID: "", + UID: uid, + Diff: diff, } } else { result.events = append(result.events, objectTmplEvalEvent{true, reasonWantFoundExists, ""}) @@ -2524,7 +2569,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( obj singleObject, objectT *policyv1.ObjectTemplate, remediation policyv1.RemediationAction, -) (throwSpecViolation bool, message string, updateNeeded bool, updateSucceeded bool) { +) (throwSpecViolation bool, message string, diff string, updateNeeded bool, updateSucceeded bool) { complianceType := strings.ToLower(string(objectT.ComplianceType)) mdComplianceType := strings.ToLower(string(objectT.MetadataComplianceType)) @@ -2552,7 +2597,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if obj.existingObj == nil { log.Info("Skipping update: Previous object retrieval from the API server failed") - return false, "", false, false + return false, "", "", false, false } var res dynamic.ResourceInterface @@ -2570,18 +2615,14 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( obj.desiredObj, obj.existingObj, existingObjectCopy, complianceType, mdComplianceType, !r.DryRunSupported, ) if message != "" { - return true, message, true, false + return true, message, "", true, false } + recordDiff := objectT.RecordDiffWithDefault() + if updateNeeded { mismatchLog := "Detected value mismatch" - // 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) // FieldValidation is supported in k8s 1.25 as beta release @@ -2590,10 +2631,12 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( if err := r.validateObject(obj.existingObj); err != nil { message := fmt.Sprintf("Error validating the object %s, the error is `%v`", obj.name, err) - return true, message, updateNeeded, false + return true, message, "", updateNeeded, false } } + isInform := remediation.IsInform() + // If the cluster supports dry run requests, verify that the API server agrees with the local comparison logic. // It's possible the dry run request shows the object does match. This can happen if the ConfigurationPolicy // specifies an empty map and the API server omits it from the return value. @@ -2610,7 +2653,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( r.setEvaluatedObject(obj.policy, obj.existingObj, false) - return true, "", false, false + return true, "", "", false, false } // If it's a conflict, refetch the object and try again. @@ -2634,7 +2677,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( ) } - return true, message, updateNeeded, false + return true, message, "", updateNeeded, false } removeFieldsForComparison(dryRunUpdatedObj) @@ -2647,36 +2690,23 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( r.setEvaluatedObject(obj.policy, obj.existingObj, true) - return false, "", false, false + return false, "", "", false, false } - // Generate and log the diff - if objectT.RecordDiff == policyv1.RecordDiffLog { - diff, err := generateDiff(existingObjectCopy, dryRunUpdatedObj) - if err != nil { - log.Info("Failed to generate the diff: " + err.Error()) - } else { - log.Info("Logging the diff:\n" + diff) - } - } - } else if objectT.RecordDiff == policyv1.RecordDiffLog { + diff = handleDiff(log, recordDiff, isInform, existingObjectCopy, dryRunUpdatedObj) + } else if recordDiff == policyv1.RecordDiffLog || (isInform && recordDiff == policyv1.RecordDiffInStatus) { // Generate and log the diff for when dryrun is unsupported (i.e. OCP v3.11) mergedObjCopy := obj.existingObj.DeepCopy() removeFieldsForComparison(mergedObjCopy) - diff, err := generateDiff(existingObjectCopy, mergedObjCopy) - if err != nil { - log.Info("Failed to generate the diff: " + err.Error()) - } else { - log.Info("Logging the diff:\n" + diff) - } + diff = handleDiff(log, recordDiff, isInform, existingObjectCopy, mergedObjCopy) } // The object would have been updated, so if it's inform, return as noncompliant. - if remediation.IsInform() { + if isInform { r.setEvaluatedObject(obj.policy, obj.existingObj, false) - return true, "", false, false + return true, "", diff, false, false } // If it's not inform (i.e. enforce), update the object @@ -2702,7 +2732,7 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( message = fmt.Sprintf("Error updating the object `%v`, the error is `%v`", obj.name, err) } - return true, message, updateNeeded, false + return true, message, diff, updateNeeded, false } if !statusMismatch { @@ -2711,10 +2741,62 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource( updateSucceeded = true } else { + if throwSpecViolation && recordDiff != policyv1.RecordDiffNone { + // The spec didn't require a change but throwSpecViolation indicates the status didn't match. Handle + // this diff for this case. + mergedObjCopy := obj.existingObj.DeepCopy() + removeFieldsForComparison(mergedObjCopy) + + // The provided isInform value is always true because the status checking can only be inform. + diff = handleDiff(log, recordDiff, true, existingObjectCopy, mergedObjCopy) + } + r.setEvaluatedObject(obj.policy, obj.existingObj, !throwSpecViolation) } - return throwSpecViolation, "", updateNeeded, updateSucceeded + return throwSpecViolation, "", diff, updateNeeded, updateSucceeded +} + +// handleDiff will generate the diff and then log it or return it based on the input recordDiff value. If recordDiff +// is set to None or is set to InStatus with enforce, no diff is generated. This is because the diff is not relevant +// after the object is updated. When recordDiff is set to Censored, a message indicating so is returned. +func handleDiff( + log logr.Logger, + recordDiff policyv1.RecordDiff, + isInform bool, + existingObject *unstructured.Unstructured, + mergedObject *unstructured.Unstructured, +) string { + if !isInform && recordDiff == policyv1.RecordDiffInStatus { + return "" + } + + var computedDiff string + + if recordDiff != policyv1.RecordDiffNone && recordDiff != policyv1.RecordDiffCensored { + var err error + + computedDiff, err = generateDiff(existingObject, mergedObject) + if err != nil { + log.Error(err, "Failed to generate the diff") + + return "" + } + } + + switch recordDiff { + case policyv1.RecordDiffNone: + return "" + case policyv1.RecordDiffLog: + log.Info("Logging the diff:\n" + computedDiff) + case policyv1.RecordDiffInStatus: + return computedDiff + case policyv1.RecordDiffCensored: + return `# This diff may contain sensitive data. The "recordDiff" field must be set to "InStatus" ` + + `to record a diff.` + } + + return "" } // handleKeys goes through all of the fields in the desired object and checks if the existing object diff --git a/controllers/configurationpolicy_utils.go b/controllers/configurationpolicy_utils.go index d6c64749..1bc89bcc 100644 --- a/controllers/configurationpolicy_utils.go +++ b/controllers/configurationpolicy_utils.go @@ -132,7 +132,8 @@ func updateRelatedObjectsStatus( currentObject.Object.Metadata.Namespace == relatedObject.Object.Metadata.Namespace { present = true - if currentObject.Compliant != relatedObject.Compliant { + if currentObject.Compliant != relatedObject.Compliant || + !reflect.DeepEqual(currentObject.Properties, relatedObject.Properties) { list[index] = relatedObject } } @@ -687,9 +688,10 @@ func generateDiff(existingObj, updatedObj *unstructured.Unstructured) (string, e return "", fmt.Errorf("failed to marshal existing object to YAML for diff: %w", err) } - existingYAMLName := existingObj.GetName() + " : existing" + name := existingObj.GetName() + if existingObj.GetNamespace() != "" { - existingYAMLName = existingObj.GetNamespace() + "/" + existingYAMLName + name = existingObj.GetNamespace() + "/" + name } updatedYAML, err := yaml.Marshal(updatedObj.Object) @@ -697,19 +699,14 @@ func generateDiff(existingObj, updatedObj *unstructured.Unstructured) (string, e return "", fmt.Errorf("failed to marshal updated object to YAML for diff: %w", err) } - updatedYAMLName := updatedObj.GetName() + " : updated" - if updatedObj.GetNamespace() != "" { - updatedYAMLName = updatedObj.GetNamespace() + "/" + updatedYAMLName - } - // Set the diffing configuration // See https://pkg.go.dev/github.com/pmezard/go-difflib/difflib#UnifiedDiff unifiedDiff := difflib.UnifiedDiff{ A: difflib.SplitLines(string(existingYAML)), - FromFile: existingYAMLName, + FromFile: name + " : existing", B: difflib.SplitLines(string(updatedYAML)), - ToFile: updatedYAMLName, - Context: 1, + ToFile: name + " : updated", + Context: 5, } // Generate and return the diff @@ -718,5 +715,13 @@ func generateDiff(existingObj, updatedObj *unstructured.Unstructured) (string, e return "", fmt.Errorf("failed to generate diff: %w", err) } + splitDiff := strings.Split(diff, "\n") + // Keep a maxmium of 50 lines of diff + 3 lines for the header + if len(splitDiff) > 53 { + diff = fmt.Sprintf( + "# Truncated: showing 50/%d diff lines:\n%s", len(splitDiff), strings.Join(splitDiff[:53], "\n"), + ) + } + return diff, nil } diff --git a/controllers/configurationpolicy_utils_test.go b/controllers/configurationpolicy_utils_test.go index 571941b7..210738f5 100644 --- a/controllers/configurationpolicy_utils_test.go +++ b/controllers/configurationpolicy_utils_test.go @@ -256,7 +256,8 @@ func TestGenerateDiff(t *testing.T) { }, }, expectedDiff: ` -@@ -2,2 +2,3 @@ +@@ -1,3 +1,4 @@ + cities: - Raleigh +- Durham`, }, @@ -273,7 +274,8 @@ func TestGenerateDiff(t *testing.T) { }, }, expectedDiff: ` -@@ -2,3 +2,2 @@ +@@ -1,4 +1,3 @@ + cities: - Raleigh -- Durham`, }, diff --git a/deploy/crds/kustomize_configurationpolicy/policy.open-cluster-management.io_configurationpolicies.yaml b/deploy/crds/kustomize_configurationpolicy/policy.open-cluster-management.io_configurationpolicies.yaml index fa4c0edf..548f7575 100644 --- a/deploy/crds/kustomize_configurationpolicy/policy.open-cluster-management.io_configurationpolicies.yaml +++ b/deploy/crds/kustomize_configurationpolicy/policy.open-cluster-management.io_configurationpolicies.yaml @@ -166,9 +166,11 @@ spec: recordDiff: description: |- RecordDiff specifies whether (and where) to log the diff between the object on the - cluster and the objectDefinition in the policy. Defaults to "None". + cluster and the objectDefinition in the policy. Defaults to "None" when the object kind is + ConfigMap, OAuthAccessToken, OAuthAuthorizeTokens, Route, or Secret. Defaults to "InStatus" otherwise. enum: - Log + - InStatus - None type: string required: @@ -318,6 +320,8 @@ spec: description: Whether the object was created by the parent policy type: boolean + diff: + type: string uid: description: Store object UID to help track object ownership for deletion diff --git a/deploy/crds/kustomize_operatorpolicy/kustomization.yaml b/deploy/crds/kustomize_operatorpolicy/kustomization.yaml index e836fde8..64eef583 100644 --- a/deploy/crds/kustomize_operatorpolicy/kustomization.yaml +++ b/deploy/crds/kustomize_operatorpolicy/kustomization.yaml @@ -15,3 +15,9 @@ patches: version: v1 kind: CustomResourceDefinition name: operatorpolicies.policy.open-cluster-management.io +- path: remove-diff.json + target: + group: apiextensions.k8s.io + version: v1 + kind: CustomResourceDefinition + name: operatorpolicies.policy.open-cluster-management.io diff --git a/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml b/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml index 02cc5890..e57c28b6 100644 --- a/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml +++ b/deploy/crds/kustomize_operatorpolicy/policy.open-cluster-management.io_operatorpolicies.yaml @@ -261,6 +261,8 @@ spec: description: Whether the object was created by the parent policy type: boolean + diff: + type: string uid: description: Store object UID to help track object ownership for deletion diff --git a/deploy/crds/kustomize_operatorpolicy/remove-diff.json b/deploy/crds/kustomize_operatorpolicy/remove-diff.json new file mode 100644 index 00000000..6ff3b461 --- /dev/null +++ b/deploy/crds/kustomize_operatorpolicy/remove-diff.json @@ -0,0 +1,6 @@ +[ + { + "op": "remove", + "path": "/spec/versions/0/schema/openAPIV3Schema/properties/status/properties/relatedObjects/items/properties/properties/properties/diff" + } +] \ No newline at end of file diff --git a/deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml b/deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml index 6be90138..6f9a6761 100644 --- a/deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml +++ b/deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml @@ -173,9 +173,11 @@ spec: recordDiff: description: |- RecordDiff specifies whether (and where) to log the diff between the object on the - cluster and the objectDefinition in the policy. Defaults to "None". + cluster and the objectDefinition in the policy. Defaults to "None" when the object kind is + ConfigMap, OAuthAccessToken, OAuthAuthorizeTokens, Route, or Secret. Defaults to "InStatus" otherwise. enum: - Log + - InStatus - None type: string required: @@ -325,6 +327,8 @@ spec: description: Whether the object was created by the parent policy type: boolean + diff: + type: string uid: description: Store object UID to help track object ownership for deletion diff --git a/go.mod b/go.mod index 32231bb1..31acfa2c 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.21 require ( github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 + github.com/go-logr/logr v1.4.1 github.com/go-logr/zapr v1.3.0 github.com/google/go-cmp v0.6.0 github.com/onsi/ginkgo/v2 v2.17.1 @@ -13,7 +14,7 @@ require ( github.com/prometheus/client_golang v1.18.0 github.com/spf13/pflag v1.0.5 github.com/stolostron/go-log-utils v0.1.2 - github.com/stolostron/go-template-utils/v4 v4.1.0 + github.com/stolostron/go-template-utils/v4 v4.2.0 github.com/stolostron/kubernetes-dependency-watches v0.6.0 github.com/stretchr/testify v1.8.4 golang.org/x/mod v0.16.0 @@ -44,7 +45,6 @@ require ( github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/go-errors/errors v1.5.1 // indirect - github.com/go-logr/logr v1.4.1 // indirect github.com/go-openapi/jsonpointer v0.20.0 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.4 // indirect diff --git a/go.sum b/go.sum index d460ad9b..27a87138 100644 --- a/go.sum +++ b/go.sum @@ -142,8 +142,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stolostron/go-log-utils v0.1.2 h1:7l1aJWvBqU2+DUyimcslT5SJpdygVY/clRDmX5sO29c= github.com/stolostron/go-log-utils v0.1.2/go.mod h1:8zrB8UJmp1rXhv3Ck9bBl5SpNfKk3SApeElbo96YRtQ= -github.com/stolostron/go-template-utils/v4 v4.1.0 h1:i7S6gTmFixZB25zY03pR5rC30PVJGDFfFuLi15Oqq6g= -github.com/stolostron/go-template-utils/v4 v4.1.0/go.mod h1:pEezMPxvuvllc68oLUejh9tfM8r/Yi7s10ZlZhVkyM4= +github.com/stolostron/go-template-utils/v4 v4.2.0 h1:HayLPKiSu3wGHjyPsplWKyEVTJ9Ju7pdzzI0/SsSPiA= +github.com/stolostron/go-template-utils/v4 v4.2.0/go.mod h1:pEezMPxvuvllc68oLUejh9tfM8r/Yi7s10ZlZhVkyM4= github.com/stolostron/kubernetes-dependency-watches v0.6.0 h1:FxXsslLNH5hNojVvKX467iefCEwigdMftfmlYX/tddg= github.com/stolostron/kubernetes-dependency-watches v0.6.0/go.mod h1:6v54aX8Bxx1m9YETZUxNGUrSHcRIArM/YnOqnUbIB/g= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/test/e2e/case20_delete_objects_test.go b/test/e2e/case20_delete_objects_test.go index 0f6d50a2..077a9461 100644 --- a/test/e2e/case20_delete_objects_test.go +++ b/test/e2e/case20_delete_objects_test.go @@ -70,7 +70,7 @@ var _ = Describe("Test Object deletion", Ordered, func() { properties := relatedObj.(map[string]interface{})["properties"].(map[string]interface{}) return properties["uid"].(string) - }, defaultTimeoutSeconds, 1).Should(Not(Equal(""))) + }, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty()) }) It("should update status fields properly for non-created objects", func() { By("Creating " + case20ConfigPolicyNameExisting + " on managed") @@ -99,7 +99,7 @@ var _ = Describe("Test Object deletion", Ordered, func() { properties := relatedObj.(map[string]interface{})["properties"].(map[string]interface{}) return properties["uid"] - }, defaultTimeoutSeconds, 1).Should(BeNil()) + }, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty()) }) It("should update status fields properly for edited objects", func() { By("Creating " + case20ConfigPolicyNameEdit + " on managed") @@ -128,7 +128,7 @@ var _ = Describe("Test Object deletion", Ordered, func() { properties := relatedObj.(map[string]interface{})["properties"].(map[string]interface{}) return properties["uid"] - }, defaultTimeoutSeconds, 1).Should(BeNil()) + }, defaultTimeoutSeconds, 1).ShouldNot(BeEmpty()) }) It("should not update status field for inform policies", func() { By("Creating " + case20ConfigPolicyNameInform + " on managed") diff --git a/test/e2e/case2_role_handling_test.go b/test/e2e/case2_role_handling_test.go index b843875c..4c73d3a8 100644 --- a/test/e2e/case2_role_handling_test.go +++ b/test/e2e/case2_role_handling_test.go @@ -4,6 +4,8 @@ package e2e import ( + "fmt" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -147,12 +149,41 @@ var _ = Describe("Test role obj template handling", Ordered, func() { plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds) Expect(plc).NotTo(BeNil()) + + var managedPlc *unstructured.Unstructured + Eventually(func() interface{} { - managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + managedPlc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, configPolicyNameBindingEnforce, testNamespace, true, defaultTimeoutSeconds) return utils.GetComplianceState(managedPlc) }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Verifying the diff in the status") + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + uid, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "uid") + Expect(uid).ToNot(BeEmpty()) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + expectedDiff := fmt.Sprintf(`--- default/pod-reader-e2e-binding : existing ++++ default/pod-reader-e2e-binding : updated +@@ -8,10 +8,6 @@ + uid: %s + roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: pod-reader-e2e +-subjects: +-- apiGroup: rbac.authorization.k8s.io +- kind: Group +- name: system:authenticated:oauth + +`, uid) + Expect(diff).To(Equal(expectedDiff)) + By("patching policy spec.remediationAction = enforce") utils.Kubectl("patch", "configurationpolicy", configPolicyNameBindingEnforce, `--type=json`, `-p=[{"op":"replace","path":"/spec/remediationAction","value":"enforce"}]`, "-n", testNamespace) diff --git a/test/e2e/case39_diff_generation_test.go b/test/e2e/case39_diff_generation_test.go index 353c1b31..c929003a 100644 --- a/test/e2e/case39_diff_generation_test.go +++ b/test/e2e/case39_diff_generation_test.go @@ -11,6 +11,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "open-cluster-management.io/config-policy-controller/test/utils" ) @@ -85,8 +86,12 @@ var _ = Describe("Generate the diff", Ordered, func() { data: - fieldToUpdate: "1" + fieldToUpdate: "2" - kind: ConfigMap - {"policy": "case39-policy-cfgmap-create", "name": "case39-map", "namespace": "default", "resource": "configmaps"}`)) + kind: ConfigMap`)) + + Expect(diff).Should(ContainSubstring( + `{"policy": "case39-policy-cfgmap-create", "name": "case39-map", "namespace": "default", ` + + `"resource": "configmaps"}`, + )) }) AfterAll(func() { @@ -94,3 +99,160 @@ var _ = Describe("Generate the diff", Ordered, func() { utils.Kubectl("delete", "configmap", "case39-map", "--ignore-not-found") }) }) + +var _ = Describe("Diff generation with sensitive input", Ordered, func() { + const ( + noDiffObjTemplatesRaw = "case39-no-diff-object-templates-raw" + noDiffObjTemplatesRawYAML = "../resources/case39_diff_generation/case39-no-diff-object-templates-raw.yaml" + noDiffObjTemplates = "case39-no-diff-object-templates" + noDiffObjTemplatesYAML = "../resources/case39_diff_generation/case39-no-diff-object-templates.yaml" + noDiffOnSecret = "case39-no-diff-on-secret" + noDiffOnSecretYAML = "../resources/case39_diff_generation/case39-no-diff-on-secret.yaml" + secretName = "case39-secret" + ) + + BeforeAll(func() { + By("Creating " + secretName + " in the default namespace") + utils.Kubectl("create", "-f", "../resources/case39_diff_generation/case39-secret.yaml") + }) + + AfterAll(func() { + deleteConfigPolicies([]string{noDiffObjTemplatesRaw, noDiffObjTemplates, noDiffOnSecret}) + utils.Kubectl("-n", "default", "delete", "secret", secretName, "--ignore-not-found") + }) + + It("Does not automatically generate a diff when using fromSecret (object-templates-raw)", func() { + By("Creating " + noDiffObjTemplatesRaw + " on managed") + utils.Kubectl("apply", "-f", noDiffObjTemplatesRawYAML, "-n", testNamespace) + + var managedPlc *unstructured.Unstructured + + Eventually(func() interface{} { + managedPlc = utils.GetWithTimeout( + clientManagedDynamic, + gvrConfigPolicy, + noDiffObjTemplatesRaw, + testNamespace, + true, + defaultTimeoutSeconds, + ) + + return utils.GetComplianceState(managedPlc) + }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Verifying the diff in the status contains instructions to set recordDiff") + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(Equal( + `# This diff may contain sensitive data. The "recordDiff" field must be set to "InStatus" ` + + `to record a diff.`, + )) + }) + + It("Does not automatically generate a diff when using fromSecret (object-templates)", func() { + By("Creating " + noDiffObjTemplates + " on managed") + utils.Kubectl("apply", "-f", noDiffObjTemplatesYAML, "-n", testNamespace) + + var managedPlc *unstructured.Unstructured + + Eventually(func() interface{} { + managedPlc = utils.GetWithTimeout( + clientManagedDynamic, + gvrConfigPolicy, + noDiffObjTemplates, + testNamespace, + true, + defaultTimeoutSeconds, + ) + + return utils.GetComplianceState(managedPlc) + }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Verifying the diff in the status contains instructions to set recordDiff") + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(Equal( + `# This diff may contain sensitive data. The "recordDiff" field must be set to "InStatus" ` + + `to record a diff.`, + )) + }) + + It("Does not automatically generate a diff when configuring a Secret", func() { + By("Creating " + noDiffOnSecret + " on managed") + utils.Kubectl("apply", "-f", noDiffOnSecretYAML, "-n", testNamespace) + + var managedPlc *unstructured.Unstructured + + Eventually(func() interface{} { + managedPlc = utils.GetWithTimeout( + clientManagedDynamic, + gvrConfigPolicy, + noDiffOnSecret, + testNamespace, + true, + defaultTimeoutSeconds, + ) + + return utils.GetComplianceState(managedPlc) + }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Verifying the diff in the status contains instructions to set recordDiff") + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(Equal( + `# This diff may contain sensitive data. The "recordDiff" field must be set to "InStatus" ` + + `to record a diff.`, + )) + }) +}) + +var _ = Describe("Diff generation that is truncated", Ordered, func() { + const ( + policyTruncatedDiff = "case39-truncated" + policyTruncatedDiffYAML = "../resources/case39_diff_generation/case39-truncated.yaml" + ) + + AfterAll(func() { + deleteConfigPolicies([]string{policyTruncatedDiff}) + }) + + It("Does not automatically generate a diff when configuring a Secret", func() { + By("Creating " + policyTruncatedDiff + " on managed") + utils.Kubectl("apply", "-f", policyTruncatedDiffYAML, "-n", testNamespace) + + var managedPlc *unstructured.Unstructured + + Eventually(func() interface{} { + managedPlc = utils.GetWithTimeout( + clientManagedDynamic, + gvrConfigPolicy, + policyTruncatedDiff, + testNamespace, + true, + defaultTimeoutSeconds, + ) + + return utils.GetComplianceState(managedPlc) + }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + By("Verifying the diff in the status is truncated") + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(HavePrefix( + "# Truncated: showing 50/68 diff lines:\n--- default : existing\n+++ default : updated", + )) + Expect(diff).To(HaveSuffix("+ message46: message")) + }) +}) diff --git a/test/e2e/case8_status_check_test.go b/test/e2e/case8_status_check_test.go index bcf4c7e5..cb7e966b 100644 --- a/test/e2e/case8_status_check_test.go +++ b/test/e2e/case8_status_check_test.go @@ -6,6 +6,7 @@ package e2e import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "open-cluster-management.io/config-policy-controller/test/utils" ) @@ -58,12 +59,22 @@ var _ = Describe("Test pod obj template handling", func() { plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, case8ConfigPolicyNameCheckFail, testNamespace, true, defaultTimeoutSeconds) Expect(plc).NotTo(BeNil()) + + var managedPlc *unstructured.Unstructured + Eventually(func() interface{} { - managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + managedPlc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, case8ConfigPolicyNameCheckFail, testNamespace, true, defaultTimeoutSeconds) return utils.GetComplianceState(managedPlc) }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(ContainSubstring("- compliant: Compliant\n+ compliant: NonCompliant")) }) It("should return nonCompliant if status does not match (enforce)", func() { By("Creating " + case8ConfigPolicyNameEnforceFail + " on managed") @@ -71,12 +82,22 @@ var _ = Describe("Test pod obj template handling", func() { plc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, case8ConfigPolicyNameEnforceFail, testNamespace, true, defaultTimeoutSeconds) Expect(plc).NotTo(BeNil()) + + var managedPlc *unstructured.Unstructured + Eventually(func() interface{} { - managedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, + managedPlc = utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy, case8ConfigPolicyNameEnforceFail, testNamespace, true, defaultTimeoutSeconds) return utils.GetComplianceState(managedPlc) }, defaultTimeoutSeconds, 1).Should(Equal("NonCompliant")) + + relatedObjects, _, err := unstructured.NestedSlice(managedPlc.Object, "status", "relatedObjects") + Expect(err).ToNot(HaveOccurred()) + Expect(relatedObjects).To(HaveLen(1)) + + diff, _, _ := unstructured.NestedString(relatedObjects[0].(map[string]interface{}), "properties", "diff") + Expect(diff).To(ContainSubstring("- compliant: Compliant\n+ compliant: NonCompliant")) }) AfterAll(func() { policies := []string{ diff --git a/test/resources/case39_diff_generation/case39-no-diff-object-templates-raw.yaml b/test/resources/case39_diff_generation/case39-no-diff-object-templates-raw.yaml new file mode 100644 index 00000000..04573788 --- /dev/null +++ b/test/resources/case39_diff_generation/case39-no-diff-object-templates-raw.yaml @@ -0,0 +1,15 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: case39-no-diff-object-templates-raw +spec: + remediationAction: inform + object-templates-raw: | + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Namespace + metadata: + name: default + annotations: + message: '{{ fromSecret "default" "case39-secret" "message" }}' diff --git a/test/resources/case39_diff_generation/case39-no-diff-object-templates.yaml b/test/resources/case39_diff_generation/case39-no-diff-object-templates.yaml new file mode 100644 index 00000000..ef806fb2 --- /dev/null +++ b/test/resources/case39_diff_generation/case39-no-diff-object-templates.yaml @@ -0,0 +1,15 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: case39-no-diff-object-templates +spec: + remediationAction: inform + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Namespace + metadata: + name: default + annotations: + message: '{{ fromSecret "default" "case39-secret" "message" }}' diff --git a/test/resources/case39_diff_generation/case39-no-diff-on-secret.yaml b/test/resources/case39_diff_generation/case39-no-diff-on-secret.yaml new file mode 100644 index 00000000..14f13a48 --- /dev/null +++ b/test/resources/case39_diff_generation/case39-no-diff-on-secret.yaml @@ -0,0 +1,17 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: case39-no-diff-on-secret +spec: + remediationAction: inform + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Secret + metadata: + name: case39-secret + namespace: default + annotations: + riddle: "If I have it, I don't share it. If I share it, I don't have it. What am I?" + hint: "See the value of the kind field" diff --git a/test/resources/case39_diff_generation/case39-secret.yaml b/test/resources/case39_diff_generation/case39-secret.yaml new file mode 100644 index 00000000..3bdadd28 --- /dev/null +++ b/test/resources/case39_diff_generation/case39-secret.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: Secret +metadata: + name: case39-secret + namespace: default +type: Opaque +data: + message: VGhlcmUncyBhIHNuYWtlIGluIG15IGJvb3Qh diff --git a/test/resources/case39_diff_generation/case39-truncated.yaml b/test/resources/case39_diff_generation/case39-truncated.yaml new file mode 100644 index 00000000..b0c98965 --- /dev/null +++ b/test/resources/case39_diff_generation/case39-truncated.yaml @@ -0,0 +1,69 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: ConfigurationPolicy +metadata: + name: case39-truncated +spec: + remediationAction: inform + object-templates: + - complianceType: musthave + objectDefinition: + apiVersion: v1 + kind: Namespace + metadata: + name: default + annotations: + message1: message + message2: message + message3: message + message4: message + message5: message + message6: message + message7: message + message8: message + message9: message + message10: message + message11: message + message12: message + message13: message + message14: message + message15: message + message16: message + message17: message + message18: message + message19: message + message20: message + message21: message + message22: message + message23: message + message24: message + message25: message + message26: message + message27: message + message28: message + message29: message + message30: message + message31: message + message32: message + message33: message + message34: message + message35: message + message36: message + message37: message + message38: message + message39: message + message40: message + message41: message + message42: message + message43: message + message44: message + message45: message + message46: message + message47: message + message48: message + message49: message + message50: message + message51: message + message52: message + message53: message + message54: message + message55: message