Skip to content

Commit

Permalink
change FieldValidation to strict
Browse files Browse the repository at this point in the history
use FieldValidation only if the k8s version is above 1.25

Signed-off-by: clyang82 <chuyang@redhat.com>
  • Loading branch information
clyang82 authored and openshift-merge-robot committed Jun 1, 2023
1 parent 1cb79a0 commit 7efe2d1
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 16 deletions.
41 changes: 34 additions & 7 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
gocmp "github.com/google/go-cmp/cmp"
"github.com/prometheus/client_golang/prometheus"
templates "github.com/stolostron/go-template-utils/v3/pkg/templates"
"golang.org/x/mod/semver"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand Down Expand Up @@ -94,6 +95,7 @@ type cachedEncryptionKey struct {
type discoveryInfo struct {
apiResourceList []*metav1.APIResourceList
apiGroups []*restmapper.APIGroupResources
serverVersion string
discoveryLastRefreshed time.Time
}

Expand Down Expand Up @@ -296,6 +298,13 @@ func (r *ConfigurationPolicyReconciler) refreshDiscoveryInfo() error {

dd := r.TargetK8sClient.Discovery()

serverVersion, err := dd.ServerVersion()
if err != nil {
log.Error(err, "Could not get the server version")
}

r.serverVersion = serverVersion.String()

_, apiResourceList, resourceErr := dd.ServerGroupsAndResources()
if resourceErr != nil {
log.Error(resourceErr, "Could not get the full API resource list")
Expand Down Expand Up @@ -2189,11 +2198,17 @@ func (r *ConfigurationPolicyReconciler) createObject(
objLog := log.WithValues("name", unstruct.GetName(), "namespace", unstruct.GetNamespace())
objLog.V(2).Info("Entered createObject", "unstruct", unstruct)

if err := r.validateObject(&unstruct); err != nil {
return nil, err
// 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 {
if err := r.validateObject(&unstruct); err != nil {
return nil, err
}
}

object, err = res.Create(context.TODO(), &unstruct, metav1.CreateOptions{})
object, err = res.Create(context.TODO(), &unstruct, metav1.CreateOptions{
FieldValidation: metav1.FieldValidationStrict,
})
if err != nil {
if k8serrors.IsAlreadyExists(err) {
objLog.V(2).Info("Got 'Already Exists' response for object")
Expand Down Expand Up @@ -2671,20 +2686,32 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
if updateNeeded {
log.V(2).Info("Updating the object based on the template definition")

if err := r.validateObject(obj.object); err != nil {
message := fmt.Sprintf("Error validating the object %s, the error is `%v`", obj.name, err)
// 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 {
if err := r.validateObject(obj.object); err != nil {
message := fmt.Sprintf("Error validating the object %s, the error is `%v`", obj.name, err)

return false, message, true, updateNeeded, false
return false, message, true, updateNeeded, false
}
}

_, err = res.Update(context.TODO(), obj.object, metav1.UpdateOptions{})
_, err = res.Update(context.TODO(), obj.object, metav1.UpdateOptions{
FieldValidation: metav1.FieldValidationStrict,
})
if k8serrors.IsNotFound(err) {
message := fmt.Sprintf("`%v` is not present and must be created", obj.object.GetKind())

return false, message, true, updateNeeded, false
}

if err != nil {
if strings.Contains(err.Error(), "strict decoding error:") {
message := fmt.Sprintf("Error validating the object %s, the error is `%v`", obj.name, err)

return false, message, true, updateNeeded, false
}

message := fmt.Sprintf("Error updating the object `%v`, the error is `%v`", obj.name, err)

return false, message, true, updateNeeded, false
Expand Down
5 changes: 3 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ require (
github.com/stolostron/go-log-utils v0.1.2
github.com/stolostron/go-template-utils/v3 v3.2.1
github.com/stretchr/testify v1.8.1
golang.org/x/mod v0.10.0
k8s.io/api v0.27.1
k8s.io/apiextensions-apiserver v0.27.1
k8s.io/apimachinery v0.27.1
k8s.io/client-go v12.0.0+incompatible
k8s.io/klog v1.0.0
k8s.io/klog/v2 v2.100.1
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f
k8s.io/kubectl v0.27.1
k8s.io/kubectl v0.0.0-00010101000000-000000000000
open-cluster-management.io/addon-framework v0.6.1
sigs.k8s.io/controller-runtime v0.14.6
sigs.k8s.io/yaml v1.3.0
Expand All @@ -36,7 +37,7 @@ require (
github.com/emicklei/go-restful/v3 v3.10.2 // indirect
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect
github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect
github.com/fsnotify/fsnotify v1.6.0 // indirect
github.com/go-errors/errors v1.0.1 // indirect
github.com/go-logr/logr v1.2.4 // indirect
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCv
github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2Wvf/TIe2xdyJxTlb6obmF18d8QdkxNDu4=
github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc=
github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d h1:105gxyaGwCFad8crR9dcMQWvV9Hvulu6hwUh4tWPJnM=
github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d/go.mod h1:ZZMPRZwes7CROmyNKgQzC3XPs6L/G2EJLHddWejkmf4=
github.com/flowstack/go-jsonschema v0.1.1/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0=
github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE=
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
Expand Down Expand Up @@ -240,6 +240,8 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk=
golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down
37 changes: 32 additions & 5 deletions test/e2e/case23_invalid_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"golang.org/x/mod/semver"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -23,12 +24,24 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
policyYAML = "../resources/case23_invalid_field/policy.yaml"
)

var serverVersion string
BeforeAll(func() {
serverVersion = utils.GetServerVersion(clientManaged)
})

It("Fails when an invalid field is provided", func() {
By("Creating the " + policyName + " policy")
utils.Kubectl("apply", "-f", policyYAML, "-n", testNamespace)

expectedMsg := "configmaps [case23] in namespace default is missing, and cannot be created, reason: " +
"`ValidationError(ConfigMap): unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap`"
"`ConfigMap in version \"v1\" cannot be handled as a ConfigMap: strict decoding error: unknown " +
"field \"invalid\"`"

// if the server version is less than 1.25.0, we use client side validation
if semver.Compare(serverVersion, "v1.25.0") < 0 {
expectedMsg = "configmaps [case23] in namespace default is missing, and cannot be created, reason: " +
"`ValidationError(ConfigMap): unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap`"
}

By("Verifying that the " + policyName + " policy is noncompliant")
Eventually(func(g Gomega) {
Expand All @@ -43,11 +56,16 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
By("Verifying events do not continue to be created after the first violation for created objects")
startTime := metav1.NewTime(time.Now())

msg := "ConfigMap in version \"v1\" cannot be handled as a ConfigMap: " +
"strict decoding error: unknown field \"invalid\""
if semver.Compare(serverVersion, "v1.25.0") < 0 {
msg = "unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap"
}
Consistently(func() interface{} {
compPlcEvents := utils.GetMatchingEvents(clientManaged, testNamespace,
policyName,
"",
"unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap",
msg,
defaultTimeoutSeconds)

if len(compPlcEvents) == 0 {
Expand All @@ -66,8 +84,13 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
_, err := clientManaged.CoreV1().ConfigMaps("default").Create(context.TODO(), configmap, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())

expectedMsg = "Error validating the object case23, the error is `ValidationError(ConfigMap): unknown " +
"field \"invalid\" in io.k8s.api.core.v1.ConfigMap`"
expectedMsg = "Error validating the object case23, the error is `ConfigMap in version \"v1\"" +
" cannot be handled as a ConfigMap: strict decoding error: unknown field \"invalid\"`"
// if the server version is less than 1.25.0, we use client side validation
if semver.Compare(serverVersion, "v1.25.0") < 0 {
expectedMsg = "Error validating the object case23, the error is `ValidationError(ConfigMap): unknown " +
"field \"invalid\" in io.k8s.api.core.v1.ConfigMap`"
}
Eventually(func() interface{} {
managedPlc := utils.GetWithTimeout(
clientManagedDynamic, gvrConfigPolicy, policyName, testNamespace, true, defaultTimeoutSeconds,
Expand All @@ -79,11 +102,15 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
By("Verifying events do not continue to be created after the first violation for existing objects")
alreadyExistsStartTime := metav1.NewTime(time.Now())

msg = "strict decoding error: unknown field \"invalid\""
if semver.Compare(serverVersion, "v1.25.0") < 0 {
msg = "unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap"
}
Consistently(func() interface{} {
compPlcEvents := utils.GetMatchingEvents(clientManaged, testNamespace,
policyName,
"",
"unknown field \"invalid\" in io.k8s.api.core.v1.ConfigMap",
msg,
defaultTimeoutSeconds)

if len(compPlcEvents) == 0 {
Expand Down
7 changes: 7 additions & 0 deletions test/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,3 +335,10 @@ func DoConfigPolicyMessageTest(clientHubDynamic dynamic.Interface,
return message
}, timeout, 1).Should(Equal(expectedMsg))
}

func GetServerVersion(clientManaged kubernetes.Interface) string {
serverVersion, err := clientManaged.Discovery().ServerVersion()
Expect(err).ToNot(HaveOccurred())

return serverVersion.String()
}

0 comments on commit 7efe2d1

Please sign in to comment.