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: Simplify admission webhook setup #1633

Merged
merged 9 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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 @@ -45,7 +45,7 @@ override:
path: ^internal/resourcelock$
- threshold: 84
path: ^internal/webhookcert$
- threshold: 87
- threshold: 85
path: ^webhook/logpipeline/v1alpha1$
- threshold: 84
path: ^webhook/logpipeline/v1beta1$
Expand Down
27 changes: 11 additions & 16 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,15 +322,8 @@ func run() error {
return fmt.Errorf("failed to enable webhook server: %w", err)
}

mgr.GetWebhookServer().Register("/validate-logpipeline", &webhook.Admission{
Handler: logpipelinewebhookv1alpha1.NewValidatingWebhookHandler(mgr.GetClient(), scheme),
})
mgr.GetWebhookServer().Register("/validate-logparser", &webhook.Admission{
Handler: logparserwebhookv1alpha1.NewValidatingWebhookHandler(scheme),
})

if err := setupMutatingWebhooks(mgr); err != nil {
return fmt.Errorf("failed to setup mutating webhooks: %w", err)
if err := setupAdmissionsWebhooks(mgr); err != nil {
return fmt.Errorf("failed to setup admission webhooks: %w", err)
}

mgr.GetWebhookServer().Register("/api/v2/alerts", selfmonitorwebhook.NewHandler(
Expand All @@ -347,37 +340,39 @@ func run() error {
return nil
}

func setupMutatingWebhooks(mgr manager.Manager) error {
if err := metricpipelinewebhookv1alpha1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
func setupAdmissionsWebhooks(mgr manager.Manager) error {
if err := metricpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup metric pipeline v1alpha1 webhook: %w", err)
}

if featureflags.IsEnabled(featureflags.V1Beta1) {
if err := metricpipelinewebhookv1beta1.SetupMetricPipelineWebhookWithManager(mgr); err != nil {
if err := metricpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup metric pipeline v1beta1 webhook: %w", err)
}
}

if err := tracepipelinewebhookv1alpha1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
if err := tracepipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup trace pipeline v1alpha1 webhook: %w", err)
}

if featureflags.IsEnabled(featureflags.V1Beta1) {
if err := tracepipelinewebhookv1beta1.SetupTracePipelineWebhookWithManager(mgr); err != nil {
if err := tracepipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup trace pipeline v1beta1 webhook: %w", err)
}
}

if err := logpipelinewebhookv1alpha1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
if err := logpipelinewebhookv1alpha1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup log pipeline v1alpha1 webhook: %w", err)
}

if featureflags.IsEnabled(featureflags.V1Beta1) {
if err := logpipelinewebhookv1beta1.SetupLogPipelineWebhookWithManager(mgr); err != nil {
if err := logpipelinewebhookv1beta1.SetupWithManager(mgr); err != nil {
return fmt.Errorf("failed to setup log pipeline v1beta1 webhook: %w", err)
}
}

logparserwebhookv1alpha1.SetupWithManager(mgr)

return nil
}

Expand Down
12 changes: 12 additions & 0 deletions webhook/logparser/v1alpha1/setup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package v1alpha1

import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

func SetupWithManager(mgr ctrl.Manager) {
mgr.GetWebhookServer().Register("/validate-logparser", &webhook.Admission{
Handler: newValidateHandler(mgr.GetScheme()),
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ import (
telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
)

type ValidatingWebhookHandler struct {
type validateHandler struct {
decoder admission.Decoder
}

func NewValidatingWebhookHandler(scheme *runtime.Scheme) *ValidatingWebhookHandler {
return &ValidatingWebhookHandler{
func newValidateHandler(scheme *runtime.Scheme) *validateHandler {
return &validateHandler{
decoder: admission.NewDecoder(scheme),
}
}

func (v *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
func (v *validateHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
log := logf.FromContext(ctx)

logParser := &telemetryv1alpha1.LogParser{}
Expand Down
18 changes: 4 additions & 14 deletions webhook/logpipeline/v1alpha1/defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,20 @@ import (
"fmt"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
)

// +kubebuilder:object:generate=false
var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
var _ webhook.CustomDefaulter = &defaulter{}

type LogPipelineDefaulter struct {
type defaulter struct {
ApplicationInputEnabled bool
ApplicationInputKeepOriginalBody bool
}

func SetupLogPipelineWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1alpha1.LogPipeline{}).
WithDefaulter(&LogPipelineDefaulter{
ApplicationInputEnabled: true,
ApplicationInputKeepOriginalBody: true,
}).
Complete()
}

func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object) error {
func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
pipeline, ok := obj.(*telemetryv1alpha1.LogPipeline)
if !ok {
return fmt.Errorf("expected an LogPipeline object but got %T", obj)
Expand All @@ -39,7 +29,7 @@ func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object)
return nil
}

func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
func (ld defaulter) applyDefaults(pipeline *telemetryv1alpha1.LogPipeline) {
if pipeline.Spec.Input.Application != nil {
if pipeline.Spec.Input.Application.Enabled == nil {
pipeline.Spec.Input.Application.Enabled = &ld.ApplicationInputEnabled
Expand Down
4 changes: 2 additions & 2 deletions webhook/logpipeline/v1alpha1/defaulter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func TestDefault(t *testing.T) {
defaulter := LogPipelineDefaulter{
sut := defaulter{
ApplicationInputEnabled: true,
ApplicationInputKeepOriginalBody: true,
}
Expand Down Expand Up @@ -67,7 +67,7 @@ func TestDefault(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := defaulter.Default(context.Background(), tt.input)
err := sut.Default(context.Background(), tt.input)
assert.NoError(t, err)
assert.Equal(t, tt.expected, tt.input)
})
Expand Down
6 changes: 3 additions & 3 deletions webhook/logpipeline/v1alpha1/files_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestDuplicateFileName(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().WithName("bar").WithFile("f1.json", "").Build()

Expand All @@ -41,7 +41,7 @@ func TestDuplicateFileNameInSamePipeline(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").WithFile("f1.json", "").Build()

Expand All @@ -61,7 +61,7 @@ func TestValidateUpdatePipeline(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().WithName("foo").WithFile("f1.json", "").Build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestValidateFirstPipeline(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().Build()

Expand All @@ -46,7 +46,7 @@ func TestValidateLimitNotExceeded(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().Build()

Expand All @@ -69,7 +69,7 @@ func TestValidateLimitExceeded(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existingPipelines...).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().Build()

Expand All @@ -89,7 +89,7 @@ func TestValidateUpdate(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

response := sut.Handle(context.Background(), admissionRequestFrom(t, existingPipeline))

Expand Down
25 changes: 25 additions & 0 deletions webhook/logpipeline/v1alpha1/setup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package v1alpha1

import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"

telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
)

func SetupWithManager(mgr ctrl.Manager) error {
if err := ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1alpha1.LogPipeline{}).
WithDefaulter(&defaulter{
ApplicationInputEnabled: true,
ApplicationInputKeepOriginalBody: true,
}).
Complete(); err != nil {
return err
}

mgr.GetWebhookServer().Register("/validate-logpipeline", &webhook.Admission{
Handler: newValidateHandler(mgr.GetClient(), mgr.GetScheme()),
})

return nil
}
2 changes: 1 addition & 1 deletion webhook/logpipeline/v1alpha1/spec_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestValidateLogPipelineSpec(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

response := sut.Handle(context.Background(), admissionRequestFrom(t, *tt.logPipeline))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@ import (
logpipelineutils "github.com/kyma-project/telemetry-manager/internal/utils/logpipeline"
)

type ValidatingWebhookHandler struct {
type validateHandler struct {
client client.Client
decoder admission.Decoder
}

func NewValidatingWebhookHandler(
func newValidateHandler(
client client.Client,
scheme *runtime.Scheme,
) *ValidatingWebhookHandler {
return &ValidatingWebhookHandler{
) *validateHandler {
return &validateHandler{
client: client,
decoder: admission.NewDecoder(scheme),
}
}

func (h *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
func (h *validateHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
log := logf.FromContext(ctx)

logPipeline := &telemetryv1alpha1.LogPipeline{}
Expand Down Expand Up @@ -64,7 +64,7 @@ func (h *ValidatingWebhookHandler) Handle(ctx context.Context, req admission.Req
return admission.Allowed("LogPipeline validation successful")
}

func (h *ValidatingWebhookHandler) validateLogPipeline(ctx context.Context, logPipeline *telemetryv1alpha1.LogPipeline) error {
func (h *validateHandler) validateLogPipeline(ctx context.Context, logPipeline *telemetryv1alpha1.LogPipeline) error {
log := logf.FromContext(ctx)

var logPipelines telemetryv1alpha1.LogPipelineList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestHandle(t *testing.T) {
logPipeline := testutils.NewLogPipelineBuilder().WithName("custom-output").WithCustomOutput("Name stdout").Build()
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

response := sut.Handle(context.Background(), admissionRequestFrom(t, logPipeline))

Expand Down Expand Up @@ -97,7 +97,7 @@ func TestHandle(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

response := sut.Handle(context.Background(), admissionRequestFrom(t, logPipeline))
require.Equal(t, tt.allowed, response.Allowed)
Expand Down
4 changes: 2 additions & 2 deletions webhook/logpipeline/v1alpha1/variable_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestVariableNotGloballyUnique(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&existingPipeline).Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().
WithName("log-pipeline-2").
Expand All @@ -48,7 +48,7 @@ func TestVariableValidator(t *testing.T) {

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects().Build()

sut := NewValidatingWebhookHandler(fakeClient, scheme)
sut := newValidateHandler(fakeClient, scheme)

newPipeline := testutils.NewLogPipelineBuilder().
WithName("log-pipeline-2").
Expand Down
12 changes: 6 additions & 6 deletions webhook/logpipeline/v1beta1/defaulter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,25 @@ import (
)

// +kubebuilder:object:generate=false
var _ webhook.CustomDefaulter = &LogPipelineDefaulter{}
var _ webhook.CustomDefaulter = &defaulter{}

type LogPipelineDefaulter struct {
type defaulter struct {
RuntimeInputEnabled bool
RuntimeInputKeepOriginalBody bool
DefaultOTLPOutputProtocol telemetryv1beta1.OTLPProtocol
}

func SetupLogPipelineWebhookWithManager(mgr ctrl.Manager) error {
func SetupWithManager(mgr ctrl.Manager) error {
skhalash marked this conversation as resolved.
Show resolved Hide resolved
return ctrl.NewWebhookManagedBy(mgr).For(&telemetryv1beta1.LogPipeline{}).
WithDefaulter(&LogPipelineDefaulter{
WithDefaulter(&defaulter{
RuntimeInputEnabled: true,
RuntimeInputKeepOriginalBody: true,
DefaultOTLPOutputProtocol: telemetryv1beta1.OTLPProtocolGRPC,
}).
Complete()
}

func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object) error {
func (ld defaulter) Default(ctx context.Context, obj runtime.Object) error {
pipeline, ok := obj.(*telemetryv1beta1.LogPipeline)
if !ok {
return fmt.Errorf("expected an LogPipeline object but got %T", obj)
Expand All @@ -41,7 +41,7 @@ func (ld LogPipelineDefaulter) Default(ctx context.Context, obj runtime.Object)
return nil
}

func (ld LogPipelineDefaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
func (ld defaulter) applyDefaults(pipeline *telemetryv1beta1.LogPipeline) {
if pipeline.Spec.Input.Runtime != nil {
if pipeline.Spec.Input.Runtime.Enabled == nil {
pipeline.Spec.Input.Runtime.Enabled = &ld.RuntimeInputEnabled
Expand Down
4 changes: 2 additions & 2 deletions webhook/logpipeline/v1beta1/defaulter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func TestDefault(t *testing.T) {
defaulter := LogPipelineDefaulter{
sut := defaulter{
RuntimeInputEnabled: true,
RuntimeInputKeepOriginalBody: true,
DefaultOTLPOutputProtocol: telemetryv1beta1.OTLPProtocolGRPC,
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestDefault(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := defaulter.Default(context.Background(), tt.input)
err := sut.Default(context.Background(), tt.input)
assert.NoError(t, err)
assert.Equal(t, tt.expected, tt.input)
})
Expand Down
Loading
Loading