Skip to content

Commit

Permalink
lib: Add ValidatingWebhookConfiguration defaulting
Browse files Browse the repository at this point in the history
This commit will add defaulting of the ValidatingWebhookConfiguration
resources. Previously, the CVO would treat these resources as generic
resources, and this would cause hotlooping.

The default values were set by values described in the Kubernetes
documentation [1].

[1] https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#validatingwebhookconfiguration-v1-admissionregistration-k8s-io
  • Loading branch information
DavidHurta committed Feb 1, 2023
1 parent bdac5ff commit 9c1a30f
Show file tree
Hide file tree
Showing 7 changed files with 649 additions and 20 deletions.
2 changes: 2 additions & 0 deletions hack/generate-lib-resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ def scheme_group_versions(types):
'github.com/openshift/api/image/v1': {'ImageStream'}, # for payload loading
'github.com/openshift/api/security/v1': {'SecurityContextConstraints'},
'github.com/operator-framework/api/pkg/operators/v1': {'OperatorGroup'},
'k8s.io/api/admissionregistration/v1': {'ValidatingWebhookConfiguration'},
'k8s.io/api/apps/v1': {'DaemonSet', 'Deployment'},
'k8s.io/api/batch/v1': {'Job'},
'k8s.io/api/core/v1': {'ConfigMap', 'Namespace', 'Service', 'ServiceAccount'},
Expand All @@ -294,6 +295,7 @@ def scheme_group_versions(types):
'github.com/openshift/api/config/v1': {'package': 'github.com/openshift/client-go/config/clientset/versioned/typed/config/v1', 'type': 'ConfigV1Client'},
'github.com/openshift/api/image/v1': {'package': 'github.com/openshift/client-go/image/clientset/versioned/typed/image/v1', 'type': 'ImageV1Client'},
'github.com/operator-framework/api/pkg/operators/v1': {'package': 'github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/typed/operators/v1', 'type': 'OperatorsV1Client'},
'k8s.io/api/admissionregistration/v1': {'package': 'k8s.io/client-go/kubernetes/typed/admissionregistration/v1', 'type': 'AdmissionregistrationV1Client'},
'k8s.io/api/apps/v1': {'package': 'k8s.io/client-go/kubernetes/typed/apps/v1', 'type': 'AppsV1Client'},
'k8s.io/api/batch/v1': {'package': 'k8s.io/client-go/kubernetes/typed/batch/v1', 'type': 'BatchV1Client'},
'k8s.io/api/core/v1': {'package': 'k8s.io/client-go/kubernetes/typed/core/v1', 'type': 'CoreV1Client'},
Expand Down
48 changes: 48 additions & 0 deletions lib/resourceapply/admissionregistration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package resourceapply

import (
"context"

"github.com/google/go-cmp/cmp"
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
admissionregv1 "k8s.io/api/admissionregistration/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
admissionregclientv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1"
"k8s.io/klog/v2"
"k8s.io/utils/pointer"
)

func ApplyValidatingWebhookConfigurationv1(ctx context.Context, client admissionregclientv1.ValidatingWebhookConfigurationsGetter, required *admissionregv1.ValidatingWebhookConfiguration, reconciling bool) (*admissionregv1.ValidatingWebhookConfiguration, bool, error) {
existing, err := client.ValidatingWebhookConfigurations().Get(ctx, required.Name, metav1.GetOptions{})
if apierrors.IsNotFound(err) {
klog.V(2).Infof("ValidatingWebhookConfiguration %s/%s not found, creating", required.Namespace, required.Name)
actual, err := client.ValidatingWebhookConfigurations().Create(ctx, required, metav1.CreateOptions{})
return actual, true, err
}
if err != nil {
return nil, false, err
}
// if we only create this resource, we have no need to continue further
if IsCreateOnly(required) {
return nil, false, nil
}

var original admissionregv1.ValidatingWebhookConfiguration
existing.DeepCopyInto(&original)
modified := pointer.Bool(false)
resourcemerge.EnsureValidatingWebhookConfiguration(modified, existing, *required)
if !*modified {
return existing, false, nil
}
if reconciling {
if diff := cmp.Diff(&original, existing); diff != "" {
klog.V(2).Infof("Updating ValidatingWebhookConfiguration %s/%s due to diff: %v", required.Namespace, required.Name, diff)
} else {
klog.V(2).Infof("Updating ValidatingWebhookConfiguration %s/%s with empty diff: possible hotloop after wrong comparison", required.Namespace, required.Name)
}
}

actual, err := client.ValidatingWebhookConfigurations().Update(ctx, existing, metav1.UpdateOptions{})
return actual, true, err
}
57 changes: 37 additions & 20 deletions lib/resourcebuilder/resourcebuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import (
"github.com/openshift/library-go/pkg/manifest"
operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
operatorsclientv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/typed/operators/v1"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
admissionregistrationclientv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1"
appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
batchclientv1 "k8s.io/client-go/kubernetes/typed/batch/v1"
coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
Expand All @@ -38,32 +40,34 @@ type builder struct {
mode Mode
modifier MetaV1ObjectModifierFunc

apiextensionsClientv1 *apiextensionsclientv1.ApiextensionsV1Client
apiregistrationClientv1 *apiregistrationclientv1.ApiregistrationV1Client
appsClientv1 *appsclientv1.AppsV1Client
batchClientv1 *batchclientv1.BatchV1Client
configClientv1 *configclientv1.ConfigV1Client
coreClientv1 *coreclientv1.CoreV1Client
imageClientv1 *imageclientv1.ImageV1Client
operatorsClientv1 *operatorsclientv1.OperatorsV1Client
rbacClientv1 *rbacclientv1.RbacV1Client
securityClientv1 *securityclientv1.SecurityV1Client
admissionregistrationClientv1 *admissionregistrationclientv1.AdmissionregistrationV1Client
apiextensionsClientv1 *apiextensionsclientv1.ApiextensionsV1Client
apiregistrationClientv1 *apiregistrationclientv1.ApiregistrationV1Client
appsClientv1 *appsclientv1.AppsV1Client
batchClientv1 *batchclientv1.BatchV1Client
configClientv1 *configclientv1.ConfigV1Client
coreClientv1 *coreclientv1.CoreV1Client
imageClientv1 *imageclientv1.ImageV1Client
operatorsClientv1 *operatorsclientv1.OperatorsV1Client
rbacClientv1 *rbacclientv1.RbacV1Client
securityClientv1 *securityclientv1.SecurityV1Client
}

func newBuilder(config *rest.Config, m manifest.Manifest) Interface {
return &builder{
raw: m.Raw,

apiextensionsClientv1: apiextensionsclientv1.NewForConfigOrDie(withProtobuf(config)),
apiregistrationClientv1: apiregistrationclientv1.NewForConfigOrDie(config),
appsClientv1: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
batchClientv1: batchclientv1.NewForConfigOrDie(withProtobuf(config)),
configClientv1: configclientv1.NewForConfigOrDie(config),
coreClientv1: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
imageClientv1: imageclientv1.NewForConfigOrDie(config),
operatorsClientv1: operatorsclientv1.NewForConfigOrDie(config),
rbacClientv1: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
securityClientv1: securityclientv1.NewForConfigOrDie(config),
admissionregistrationClientv1: admissionregistrationclientv1.NewForConfigOrDie(withProtobuf(config)),
apiextensionsClientv1: apiextensionsclientv1.NewForConfigOrDie(withProtobuf(config)),
apiregistrationClientv1: apiregistrationclientv1.NewForConfigOrDie(config),
appsClientv1: appsclientv1.NewForConfigOrDie(withProtobuf(config)),
batchClientv1: batchclientv1.NewForConfigOrDie(withProtobuf(config)),
configClientv1: configclientv1.NewForConfigOrDie(config),
coreClientv1: coreclientv1.NewForConfigOrDie(withProtobuf(config)),
imageClientv1: imageclientv1.NewForConfigOrDie(config),
operatorsClientv1: operatorsclientv1.NewForConfigOrDie(config),
rbacClientv1: rbacclientv1.NewForConfigOrDie(withProtobuf(config)),
securityClientv1: securityclientv1.NewForConfigOrDie(config),
}
}

Expand Down Expand Up @@ -119,6 +123,18 @@ func (b *builder) Do(ctx context.Context) error {
return err
}
}
case *admissionregistrationv1.ValidatingWebhookConfiguration:
if b.modifier != nil {
b.modifier(typedObject)
}
if deleteReq, err := resourcedelete.DeleteValidatingWebhookConfigurationv1(ctx, b.admissionregistrationClientv1, typedObject,
updatingMode); err != nil {
return err
} else if !deleteReq {
if _, _, err := resourceapply.ApplyValidatingWebhookConfigurationv1(ctx, b.admissionregistrationClientv1, typedObject, reconcilingMode); err != nil {
return err
}
}
case *appsv1.DaemonSet:
if b.modifier != nil {
b.modifier(typedObject)
Expand Down Expand Up @@ -286,6 +302,7 @@ func (b *builder) Do(ctx context.Context) error {

func init() {
rm := NewResourceMapper()
rm.RegisterGVK(admissionregistrationv1.SchemeGroupVersion.WithKind("ValidatingWebhookConfiguration"), newBuilder)
rm.RegisterGVK(apiextensionsv1.SchemeGroupVersion.WithKind("CustomResourceDefinition"), newBuilder)
rm.RegisterGVK(appsv1.SchemeGroupVersion.WithKind("DaemonSet"), newBuilder)
rm.RegisterGVK(appsv1.SchemeGroupVersion.WithKind("Deployment"), newBuilder)
Expand Down
41 changes: 41 additions & 0 deletions lib/resourcedelete/admissionregistration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package resourcedelete

import (
"context"
"fmt"

admissionregv1 "k8s.io/api/admissionregistration/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
admissionregclientv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1"
)

// DeleteValidatingWebhookConfigurationv1 checks the given resource for a valid delete
// annotation. If found it checks the status of a previously issued delete request.
// If delete has not been requested and in UpdatingMode it will issue a delete request.
func DeleteValidatingWebhookConfigurationv1(ctx context.Context,
client admissionregclientv1.ValidatingWebhookConfigurationsGetter,
required *admissionregv1.ValidatingWebhookConfiguration,
updateMode bool) (bool, error) {

if delAnnoFound, err := ValidDeleteAnnotation(required.Annotations); !delAnnoFound || err != nil {
return delAnnoFound, err
}
existing, err := client.ValidatingWebhookConfigurations().Get(ctx, required.Name, metav1.GetOptions{})
resource := Resource{
Kind: "validatingwebhookconfiguration",
Namespace: required.Namespace,
Name: required.Name,
}
if deleteRequested, err := GetDeleteProgress(resource, err); err == nil {
// Only request deletion when in update mode.
if !deleteRequested && updateMode {
if err := client.ValidatingWebhookConfigurations().Delete(ctx, required.Name, metav1.DeleteOptions{}); err != nil {
return true, fmt.Errorf("Delete request for %s failed, err=%v", resource, err)
}
SetDeleteRequested(existing, resource)
}
} else {
return true, fmt.Errorf("Error running delete for %s, err=%v", resource, err)
}
return true, nil
}
56 changes: 56 additions & 0 deletions lib/resourcemerge/admissionregistration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package resourcemerge

import (
admissionregv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
)

// EnsureValidatingWebhookConfiguration ensures that the existing matches the required.
// modified is set to true when existing had to be updated with required.
func EnsureValidatingWebhookConfiguration(modified *bool, existing *admissionregv1.ValidatingWebhookConfiguration, required admissionregv1.ValidatingWebhookConfiguration) {
EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta)
ensureValidatingWebhookConfigurationDefaults(&required)
if !equality.Semantic.DeepEqual(existing.Webhooks, required.Webhooks) {
*modified = true
existing.Webhooks = required.Webhooks
}
}

func ensureValidatingWebhookConfigurationDefaults(required *admissionregv1.ValidatingWebhookConfiguration) {
for i := range required.Webhooks {
ensureValidatingWebhookDefaults(&required.Webhooks[i])
}
}

func ensureValidatingWebhookDefaults(required *admissionregv1.ValidatingWebhook) {
if required.FailurePolicy == nil {
policy := admissionregv1.Fail
required.FailurePolicy = &policy
}
if required.MatchPolicy == nil {
policy := admissionregv1.Equivalent
required.MatchPolicy = &policy
}
if required.NamespaceSelector == nil {
required.NamespaceSelector = &metav1.LabelSelector{}
}
if required.ObjectSelector == nil {
required.ObjectSelector = &metav1.LabelSelector{}
}
if required.TimeoutSeconds == nil {
required.TimeoutSeconds = pointer.Int32(10)
}
for i := range required.Rules {
ensureRuleWithOperationsDefaults(&required.Rules[i])
}
}

func ensureRuleWithOperationsDefaults(required *admissionregv1.RuleWithOperations) {
if required.Scope == nil {
scope := admissionregv1.AllScopes
required.Scope = &scope
}
}

Loading

0 comments on commit 9c1a30f

Please sign in to comment.