Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Migrate validating webhook to kustomize #1617

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
34de814
migrate validating webhook to kustomize
shorim Nov 19, 2024
e53b64d
rename validating webhook in tests
shorim Nov 19, 2024
18a1a94
remove deletion of validating webhook from Telemetry reconciler
shorim Nov 19, 2024
1d0b871
delete old validating webhook
shorim Nov 19, 2024
e38b04d
fix linting
shorim Nov 19, 2024
b2e5773
adjust telemetry e2e test
shorim Nov 19, 2024
d645b0d
adjust overrides e2e test
shorim Nov 19, 2024
3cffe8a
Merge remote-tracking branch 'origin/main' into migrate_validating_we…
shorim Nov 19, 2024
51f3c9c
adjust unit tests
shorim Nov 19, 2024
0dd5e52
delete unnecessary unit test
shorim Nov 19, 2024
8e4e9f7
use update request instead of patch request for conversion webhook
shorim Nov 19, 2024
0b79afa
rename validatin webhook
shorim Nov 20, 2024
f725b9d
rename variable for ValidatingWebhookName in e2e tests common names
shorim Nov 20, 2024
d7e5b7a
rename logpipeline and logparsers webhooks
shorim Nov 20, 2024
66ba7c3
fix linting
shorim Nov 20, 2024
b0477b0
adjust test coverage threshold for internal/webhookcert pkg
shorim Nov 20, 2024
643fbd4
update clusterRole for manager to allow updating CRDs
shorim Nov 20, 2024
7addc21
Merge branch 'main' into migrate_validating_webhook_to_kustomize
shorim Nov 20, 2024
f17ef1d
rename updateLogPipelineWithWebhookConfig func
shorim Nov 20, 2024
ccb1863
Merge remote-tracking branch 'origin/main' into migrate_validating_we…
shorim Nov 20, 2024
ce48871
rename unnecessary WebhookHealthy assertion
shorim Nov 20, 2024
5edaff2
Merge remote-tracking branch 'origin/main' into migrate_validating_we…
shorim Nov 20, 2024
1a135dc
Merge remote-tracking branch 'origin/main' into migrate_validating_we…
shorim Nov 20, 2024
c91fc5d
Merge branch 'main' into migrate_validating_webhook_to_kustomize
skhalash Nov 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .testcoverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ override:
path: ^internal/validators/secretref$
- threshold: 75
path: ^internal/resourcelock$
- threshold: 85
- threshold: 84
path: ^internal/webhookcert$
- threshold: 73
path: ^webhook/logpipeline$
Expand Down
2 changes: 1 addition & 1 deletion config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ rules:
verbs:
- get
- list
- patch
- update
- watch
- apiGroups:
- apps
Expand Down
2 changes: 1 addition & 1 deletion config/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
resources:
#- manifests.yaml
- manifests.yaml
- service.yaml

configurations:
Expand Down
10 changes: 5 additions & 5 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
name: validation.webhook.telemetry.kyma-project.io
name: validating-webhook.kyma-project.io
webhooks:
- admissionReviewVersions:
- v1beta1
- v1
clientConfig:
service:
name: telemetry-manager-webhook
namespace: system
namespace: kyma-system
path: /validate-logpipeline
port: 443
failurePolicy: Fail
matchPolicy: Exact
name: validation.logpipelines.telemetry.kyma-project.io
name: validating-logpipelines.kyma-project.io
namespaceSelector: {}
objectSelector: {}
rules:
Expand All @@ -38,12 +38,12 @@ webhooks:
clientConfig:
service:
name: telemetry-manager-webhook
namespace: system
namespace: kyma-system
path: /validate-logparser
port: 443
failurePolicy: Fail
matchPolicy: Exact
name: validating.logparsers.telemetry.kyma-project.io
name: validating-logparsers.kyma-project.io
namespaceSelector: {}
objectSelector: {}
rules:
Expand Down
1 change: 0 additions & 1 deletion config/webhook/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ apiVersion: v1
kind: Service
metadata:
name: manager-webhook
namespace: system
spec:
ports:
- port: 443
Expand Down
37 changes: 22 additions & 15 deletions internal/reconciler/telemetry/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
}

func (r *Reconciler) doReconcile(ctx context.Context, telemetry *operatorv1alpha1.Telemetry) error {
if err := r.deleteOldValidatingWebhook(ctx); err != nil {
return fmt.Errorf("failed to delete old validating webhook: %w", err)
}

if err := r.handleFinalizer(ctx, telemetry); err != nil {
return fmt.Errorf("failed to manage finalizer: %w", err)
}
Expand Down Expand Up @@ -255,11 +259,6 @@ func (r *Reconciler) handleFinalizer(ctx context.Context, telemetry *operatorv1a
return nil
}

err := r.deleteWebhook(ctx)
if err != nil && !apierrors.IsNotFound(err) {
return fmt.Errorf("failed to delete webhook: %w", err)
}

controllerutil.RemoveFinalizer(telemetry, finalizer)

if err := r.Update(ctx, telemetry); err != nil {
Expand All @@ -270,16 +269,6 @@ func (r *Reconciler) handleFinalizer(ctx context.Context, telemetry *operatorv1a
return nil
}

func (r *Reconciler) deleteWebhook(ctx context.Context) error {
webhook := &admissionregistrationv1.ValidatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: r.config.Webhook.CertConfig.WebhookName.Name,
},
}

return r.Delete(ctx, webhook)
}

func (r *Reconciler) reconcileWebhook(ctx context.Context, telemetry *operatorv1alpha1.Telemetry) error {
// We skip webhook reconciliation only if no pipelines are remaining. This avoids the risk of certificate expiration while waiting for deletion.
if !telemetry.DeletionTimestamp.IsZero() && !r.dependentCRsFound(ctx) {
Expand Down Expand Up @@ -314,3 +303,21 @@ func (r *Reconciler) reconcileWebhook(ctx context.Context, telemetry *operatorv1

return nil
}

func (r *Reconciler) deleteOldValidatingWebhook(ctx context.Context) error {
oldValidatingWebhook := &admissionregistrationv1.ValidatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "validation.webhook.telemetry.kyma-project.io",
},
}

if err := r.Delete(ctx, oldValidatingWebhook); err != nil {
if apierrors.IsNotFound(err) {
return nil
}

return fmt.Errorf("failed to delete old validating webhook: %w", err)
}

return nil
}
85 changes: 16 additions & 69 deletions internal/webhookcert/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,92 +6,41 @@ import (

admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

k8sutils "github.com/kyma-project/telemetry-manager/internal/utils/k8s"
)

const (
webhookServicePort int32 = 443
)

// applyWebhookConfigResources creates or updates a ValidatingWebhookConfiguration for the LogPipeline/LogParser resources.
// additionally it patches a LogPipeline conversion webhook configuration.
// applyWebhookConfigResources applies the following webhook configurations:
// 1- Updates validating webhook configuration with the provided CA bundle.
// 2- Updates LogPipeline CRD with conversion webhook configuration.
func applyWebhookConfigResources(ctx context.Context, c client.Client, caBundle []byte, config Config) error {
validatingWebhookConfig := makeValidatingWebhookConfig(caBundle, config)
if err := k8sutils.CreateOrUpdateValidatingWebhookConfiguration(ctx, c, &validatingWebhookConfig); err != nil {
return fmt.Errorf("failed to create or update validating webhook configuration: %w", err)
if err := updateValidatingWebhookConfig(ctx, c, caBundle, config); err != nil {
return fmt.Errorf("failed to update validating webhook with CA bundle: %w", err)
}

conversionWebhookConfig := makeConversionWebhookConfig(caBundle, config)
if err := patchConversionWebhookConfig(ctx, c, conversionWebhookConfig); err != nil {
return fmt.Errorf("failed to patch conversion webhook configuration: %w", err)
if err := updateLogPipelineWithWebhookConfig(ctx, c, conversionWebhookConfig); err != nil {
return fmt.Errorf("failed to update LogPipeline CRD with conversion webhook configuration: %w", err)
}

return nil
}

func makeValidatingWebhookConfig(caBundle []byte, config Config) admissionregistrationv1.ValidatingWebhookConfiguration {
apiGroups := []string{"telemetry.kyma-project.io"}
apiVersions := []string{"v1alpha1"}
webhookTimeout := int32(15) //nolint:mnd // 15 seconds
labels := map[string]string{
"control-plane": "telemetry-manager",
"app.kubernetes.io/instance": "telemetry",
"app.kubernetes.io/name": "manager",
"kyma-project.io/component": "controller",
func updateValidatingWebhookConfig(ctx context.Context, c client.Client, caBundle []byte, config Config) error {
var validatingWebhookConfig admissionregistrationv1.ValidatingWebhookConfiguration
if err := c.Get(ctx, config.WebhookName, &validatingWebhookConfig); err != nil {
return fmt.Errorf("failed to get validating webhook configuration: %w", err)
}

createWebhook := func(name, path string, resources []string) admissionregistrationv1.ValidatingWebhook {
return admissionregistrationv1.ValidatingWebhook{
AdmissionReviewVersions: []string{"v1beta1", "v1"},
ClientConfig: admissionregistrationv1.WebhookClientConfig{
Service: &admissionregistrationv1.ServiceReference{
Name: config.ServiceName.Name,
Namespace: config.ServiceName.Namespace,
Port: ptr.To(webhookServicePort),
Path: &path,
},
CABundle: caBundle,
},
FailurePolicy: ptr.To(admissionregistrationv1.Fail),
MatchPolicy: ptr.To(admissionregistrationv1.Exact),
Name: name,
SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone),
TimeoutSeconds: &webhookTimeout,
Rules: []admissionregistrationv1.RuleWithOperations{
{
Operations: []admissionregistrationv1.OperationType{
admissionregistrationv1.Create,
admissionregistrationv1.Update,
},
Rule: admissionregistrationv1.Rule{
APIGroups: apiGroups,
APIVersions: apiVersions,
Scope: ptr.To(admissionregistrationv1.AllScopes),
Resources: resources,
},
},
},
}
}

webhooks := []admissionregistrationv1.ValidatingWebhook{
createWebhook("validation.logpipelines.telemetry.kyma-project.io", "/validate-logpipeline", []string{"logpipelines"}),
createWebhook("validation.logparsers.telemetry.kyma-project.io", "/validate-logparser", []string{"logparsers"}),
}
validatingWebhookConfig.Webhooks[0].ClientConfig.CABundle = caBundle
validatingWebhookConfig.Webhooks[1].ClientConfig.CABundle = caBundle

return admissionregistrationv1.ValidatingWebhookConfiguration{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: config.WebhookName.Name,
Labels: labels,
},
Webhooks: webhooks,
}
return c.Update(ctx, &validatingWebhookConfig)
}

func makeConversionWebhookConfig(caBundle []byte, config Config) apiextensionsv1.CustomResourceConversion {
Expand All @@ -112,15 +61,13 @@ func makeConversionWebhookConfig(caBundle []byte, config Config) apiextensionsv1
}
}

func patchConversionWebhookConfig(ctx context.Context, c client.Client, conversion apiextensionsv1.CustomResourceConversion) error {
func updateLogPipelineWithWebhookConfig(ctx context.Context, c client.Client, conversion apiextensionsv1.CustomResourceConversion) error {
shorim marked this conversation as resolved.
Show resolved Hide resolved
var logPipelineCRD apiextensionsv1.CustomResourceDefinition
if err := c.Get(ctx, types.NamespacedName{Name: "logpipelines.telemetry.kyma-project.io"}, &logPipelineCRD); err != nil {
return fmt.Errorf("failed to get logpipelines CRD: %w", err)
}

patch := client.MergeFrom(logPipelineCRD.DeepCopy())

logPipelineCRD.Spec.Conversion = &conversion

return c.Patch(ctx, &logPipelineCRD, patch)
return c.Update(ctx, &logPipelineCRD)
}
Loading
Loading