Skip to content

Commit

Permalink
Bug: ACM-5052, Policy gets shortly into non-compliant state
Browse files Browse the repository at this point in the history
Solution: decode automatically converted stringData prior to evaluation

Signed-off-by: Jeffrey Luo <jeluo@redhat.com>
  • Loading branch information
JeffeyL authored and openshift-merge-robot committed Jun 28, 2023
1 parent 60761a4 commit 2bd6142
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 0 deletions.
25 changes: 25 additions & 0 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package controllers

import (
"context"
"encoding/base64"
"errors"
"fmt"
"reflect"
Expand Down Expand Up @@ -2462,6 +2463,30 @@ func handleSingleKey(
mergedValue.(map[string]interface{}), existingValue.(map[string]interface{}))
}

if key == "stringData" && existingObj.GetKind() == "Secret" {
// override automatic conversion from stringData to data prior to evaluation
existingValue = existingObj.UnstructuredContent()["data"]

encodedValue, _, err := unstructured.NestedStringMap(existingObj.Object, "data")
if err != nil {
message := "Error accessing encoded data"

return message, false, mergedValue, false
}

for k, value := range encodedValue {
decodedVal, err := base64.StdEncoding.DecodeString(value)
if err != nil {
secretName := existingObj.GetName()
message := fmt.Sprintf("Error decoding secret: %s", secretName)

return message, false, mergedValue, false
}

existingValue.(map[string]interface{})[k] = string(decodedVal)
}
}

// sort objects before checking equality to ensure they're in the same order
if !equalObjWithSort(mergedValue, existingValue) {
updateNeeded = true
Expand Down
66 changes: 66 additions & 0 deletions test/e2e/case32_secret_stringdata_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright Contributors to the Open Cluster Management project

package e2e

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"open-cluster-management.io/config-policy-controller/test/utils"
)

const (
case32ConfigPolicyName string = "case32config"
case32CreatePolicyYaml string = "../resources/case32_secret_stringdata/case32_create_secret.yaml"
)

var _ = Describe("Test converted stringData being decoded before comparison for Secrets", Ordered, func() {
It("Config should be created properly on the managed cluster", func() {
By("Creating " + case32ConfigPolicyName + " on managed")
utils.Kubectl("apply", "-f", case32CreatePolicyYaml, "-n", testNamespace)
cfg := utils.GetWithTimeout(clientManagedDynamic, gvrConfigPolicy,
case32ConfigPolicyName, testNamespace, true, defaultTimeoutSeconds)
Expect(cfg).NotTo(BeNil())
})

It("Verifies the config policy is initially compliant "+case32ConfigPolicyName+" in "+testNamespace, func() {
By("Waiting for " + case32ConfigPolicyName + " to become Compliant")
Eventually(func() interface{} {
cfgplc := utils.GetWithTimeout(
clientManagedDynamic, gvrConfigPolicy,
case32ConfigPolicyName, testNamespace,
true, defaultTimeoutSeconds,
)

return utils.GetComplianceState(cfgplc)
}, defaultTimeoutSeconds, 1).Should(Equal("Compliant"))
})

It("Verifies that a secret is created by "+case32ConfigPolicyName+" in openshift-config", func() {
By("Grabbing htpasswd-secret from namespace openshift-config")
scrt := utils.GetWithTimeout(
clientManagedDynamic, gvrSecret, "htpasswd-secret", testNamespace, true, defaultTimeoutSeconds,
)

Expect(scrt).NotTo(BeNil())
})

It("Verifies the config policy "+case32ConfigPolicyName+" does not update in "+testNamespace, func() {
By("Checking the events on the configuration policy" + case32ConfigPolicyName)
Consistently(func() int {
eventlen := len(utils.GetMatchingEvents(
clientManaged, testNamespace,
case32ConfigPolicyName, case32ConfigPolicyName,
"updated", defaultTimeoutSeconds))

return eventlen
}, 60, 2).Should(BeNumerically("<", 1))
})

AfterAll(func() {
utils.Kubectl("delete", "configurationpolicy", case32ConfigPolicyName, "-n", testNamespace)
utils.Kubectl("delete", "secret", "htpasswd-secret", "-n", testNamespace)
utils.Kubectl("delete", "event",
"--field-selector=involvedObject.name="+case32ConfigPolicyName, "-n", "managed")
})
})
21 changes: 21 additions & 0 deletions test/resources/case32_secret_stringdata/case32_create_secret.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
apiVersion: policy.open-cluster-management.io/v1
kind: ConfigurationPolicy
metadata:
name: case32config
spec:
object-templates:
- complianceType: musthave
objectDefinition:
apiVersion: v1
kind: Secret
metadata:
name: htpasswd-secret
namespace: managed
stringData:
htpasswd: |
alice:$QGJM$2qku.rrSEWRFD/tjOf.dZQ1q.RaR5tTmfNyZn0$1uI6eDx/Ont37ws
bob:S1KPovd0N/iDb9$dlTBH7weXG5rQV$5EPf9KrbJjw2HXy35ujhPw2y0$SIto
cadence:W$MeWCMWuKi0ApUyZ8byG5$DNL.3FD$c72kFhyZIX8Sf0yJH2AEOGFsRgEHJ
pruneObjectBehavior: None
remediationAction: enforce
severity: medium

0 comments on commit 2bd6142

Please sign in to comment.