From a794d0565d54a560ae1b6b0779c0785365401570 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Wed, 4 Oct 2023 15:46:37 -0700 Subject: [PATCH] fix webhooks no longer self-register decoders --- src/go/k8s/cmd/main.go | 13 +- .../k8s/webhooks/redpanda/console_webhook.go | 26 +-- .../redpanda/validate_enterprise_test.go | 158 +++++++++++------- 3 files changed, 108 insertions(+), 89 deletions(-) diff --git a/src/go/k8s/cmd/main.go b/src/go/k8s/cmd/main.go index 9f38267b202cd..d1845a107c8b9 100644 --- a/src/go/k8s/cmd/main.go +++ b/src/go/k8s/cmd/main.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterredpandacomv1alpha1 "github.com/redpanda-data/redpanda/src/go/k8s/api/cluster.redpanda.com/v1alpha1" redpandav1alpha1 "github.com/redpanda-data/redpanda/src/go/k8s/api/redpanda/v1alpha1" @@ -289,8 +290,16 @@ func main() { os.Exit(1) } hookServer := mgr.GetWebhookServer() - hookServer.Register("/mutate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{Handler: &redpandawebhooks.ConsoleDefaulter{Client: mgr.GetClient()}}) - hookServer.Register("/validate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{Handler: &redpandawebhooks.ConsoleValidator{Client: mgr.GetClient()}}) + hookServer.Register("/mutate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{ + Handler: &redpandawebhooks.ConsoleDefaulter{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(scheme), + }}) + hookServer.Register("/validate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{ + Handler: &redpandawebhooks.ConsoleValidator{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(scheme), + }}) } case OperatorV2Mode: ctrl.Log.Info("running in v2", "mode", OperatorV2Mode, "namespace", namespace) diff --git a/src/go/k8s/webhooks/redpanda/console_webhook.go b/src/go/k8s/webhooks/redpanda/console_webhook.go index 50f2f4d1f21e6..2a5f87f75af76 100644 --- a/src/go/k8s/webhooks/redpanda/console_webhook.go +++ b/src/go/k8s/webhooks/redpanda/console_webhook.go @@ -27,7 +27,7 @@ var validHostnameSegment = regexp.MustCompile(`^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0 // ConsoleValidator validates Consoles type ConsoleValidator struct { Client client.Client - decoder *admission.Decoder + Decoder *admission.Decoder } // Handle processes admission for Console @@ -37,7 +37,7 @@ func (v *ConsoleValidator) Handle( ) admission.Response { console := &vectorizedv1alpha1.Console{} - err := v.decoder.Decode(req, console) + err := v.Decoder.Decode(req, console) if err != nil { return admission.Errored(http.StatusBadRequest, err) } @@ -97,21 +97,12 @@ func (v *ConsoleValidator) Handle( return admission.Allowed("") } -// ConsoleValidator implements admission.DecoderInjector. -// A decoder will be automatically injected. - -// InjectDecoder injects the decoder. -func (v *ConsoleValidator) InjectDecoder(d *admission.Decoder) error { - v.decoder = d - return nil -} - // +kubebuilder:webhook:path=/mutate-redpanda-vectorized-io-v1alpha1-console,mutating=true,failurePolicy=fail,sideEffects=None,groups="redpanda.vectorized.io",resources=consoles,verbs=create;update,versions=v1alpha1,name=mconsole.kb.io,admissionReviewVersions=v1 // ConsoleDefaulter mutates Consoles type ConsoleDefaulter struct { Client client.Client - decoder *admission.Decoder + Decoder *admission.Decoder } // Handle processes admission for Console @@ -121,7 +112,7 @@ func (m *ConsoleDefaulter) Handle( ) admission.Response { console := &vectorizedv1alpha1.Console{} - err := m.decoder.Decode(req, console) + err := m.Decoder.Decode(req, console) if err != nil { return admission.Errored(http.StatusBadRequest, err) } @@ -168,12 +159,3 @@ func (m *ConsoleDefaulter) Default( response := admission.PatchResponseFromRaw(original, current) return &response, nil } - -// ConsoleDefaulter implements admission.DecoderInjector. -// A decoder will be automatically injected. - -// InjectDecoder injects the decoder. -func (m *ConsoleDefaulter) InjectDecoder(d *admission.Decoder) error { - m.decoder = d - return nil -} diff --git a/src/go/k8s/webhooks/redpanda/validate_enterprise_test.go b/src/go/k8s/webhooks/redpanda/validate_enterprise_test.go index 02bfe7ba5e395..43ecf98c1cdc5 100644 --- a/src/go/k8s/webhooks/redpanda/validate_enterprise_test.go +++ b/src/go/k8s/webhooks/redpanda/validate_enterprise_test.go @@ -9,20 +9,28 @@ import ( "testing" "time" + cmapiv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + helmControllerAPIV2 "github.com/fluxcd/helm-controller/api/v2beta1" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/stretchr/testify/require" "github.com/twmb/franz-go/pkg/kadm" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/scheme" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/envtest" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/redpanda-data/redpanda/src/go/k8s/api/vectorized/v1alpha1" + clusterredpandacomv1alpha1 "github.com/redpanda-data/redpanda/src/go/k8s/api/cluster.redpanda.com/v1alpha1" + redpandav1alpha1 "github.com/redpanda-data/redpanda/src/go/k8s/api/redpanda/v1alpha1" + vectorizedv1alpha1 "github.com/redpanda-data/redpanda/src/go/k8s/api/vectorized/v1alpha1" redpandacontrollers "github.com/redpanda-data/redpanda/src/go/k8s/internal/controller/redpanda" adminutils "github.com/redpanda-data/redpanda/src/go/k8s/pkg/admin" consolepkg "github.com/redpanda-data/redpanda/src/go/k8s/pkg/console" @@ -32,6 +40,23 @@ import ( type mockKafkaAdmin struct{} +var ( + scheme = runtime.NewScheme() +) + +//nolint:wsl // the init was generated by kubebuilder +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(cmapiv1.AddToScheme(scheme)) + utilruntime.Must(helmControllerAPIV2.AddToScheme(scheme)) + utilruntime.Must(sourcev1.AddToScheme(scheme)) + + utilruntime.Must(redpandav1alpha1.AddToScheme(scheme)) + utilruntime.Must(vectorizedv1alpha1.AddToScheme(scheme)) + utilruntime.Must(clusterredpandacomv1alpha1.AddToScheme(scheme)) + //+kubebuilder:scaffold:scheme +} + func (m *mockKafkaAdmin) CreateACLs( context.Context, *kadm.ACLBuilder, ) (kadm.CreateACLsResults, error) { @@ -58,16 +83,11 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { require.NoError(t, err) require.NotNil(t, cfg) - err = scheme.AddToScheme(scheme.Scheme) - require.NoError(t, err) - err = v1alpha1.AddToScheme(scheme.Scheme) - require.NoError(t, err) - testAdminAPI := &adminutils.MockAdminAPI{Log: ctrl.Log.WithName("testAdminAPI").WithName("mockAdminAPI")} testAdminAPIFactory := func( _ context.Context, _ client.Reader, - _ *v1alpha1.Cluster, + _ *vectorizedv1alpha1.Cluster, _ string, _ types.AdminTLSConfigProvider, ordinals ...int32, @@ -88,7 +108,7 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { CertDir: webhookInstallOptions.LocalServingCertDir, }) mgr, err := ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, + Scheme: scheme, WebhookServer: webhookServer, LeaderElection: false, Metrics: metricsserver.Options{ @@ -99,11 +119,11 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { testStore := consolepkg.NewStore(mgr.GetClient(), mgr.GetScheme()) testKafkaAdmin := &mockKafkaAdmin{} - testKafkaAdminFactory := func(context.Context, client.Client, *v1alpha1.Cluster, *consolepkg.Store) (consolepkg.KafkaAdminClient, error) { + testKafkaAdminFactory := func(context.Context, client.Client, *vectorizedv1alpha1.Cluster, *consolepkg.Store) (consolepkg.KafkaAdminClient, error) { return testKafkaAdmin, nil } - err = (&v1alpha1.Cluster{}).SetupWebhookWithManager(mgr) + err = (&vectorizedv1alpha1.Cluster{}).SetupWebhookWithManager(mgr) require.NoError(t, err) hookServer := mgr.GetWebhookServer() @@ -118,8 +138,16 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { }).WithClusterDomain("").SetupWithManager(mgr) require.NoError(t, err) - hookServer.Register("/mutate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{Handler: &redpanda.ConsoleDefaulter{Client: mgr.GetClient()}}) - hookServer.Register("/validate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{Handler: &redpanda.ConsoleValidator{Client: mgr.GetClient()}}) + hookServer.Register("/mutate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{ + Handler: &redpanda.ConsoleDefaulter{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(scheme), + }}) + hookServer.Register("/validate-redpanda-vectorized-io-v1alpha1-console", &webhook.Admission{ + Handler: &redpanda.ConsoleValidator{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(scheme), + }}) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -144,24 +172,24 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { conn.Close() - c, err := client.New(testEnv.Config, client.Options{Scheme: scheme.Scheme}) + c, err := client.New(testEnv.Config, client.Options{Scheme: scheme}) require.NoError(t, err) one := int32(1) - cluster := v1alpha1.Cluster{ + cluster := vectorizedv1alpha1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster", Namespace: "default", }, - Spec: v1alpha1.ClusterSpec{ + Spec: vectorizedv1alpha1.ClusterSpec{ Replicas: &one, - Configuration: v1alpha1.RedpandaConfig{ - KafkaAPI: []v1alpha1.KafkaAPI{ + Configuration: vectorizedv1alpha1.RedpandaConfig{ + KafkaAPI: []vectorizedv1alpha1.KafkaAPI{ { Port: 9091, }, }, - AdminAPI: []v1alpha1.AdminAPI{ + AdminAPI: []vectorizedv1alpha1.AdminAPI{ { Port: 8080, }, @@ -170,37 +198,37 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { }, } - console := v1alpha1.Console{ + console := vectorizedv1alpha1.Console{ ObjectMeta: metav1.ObjectMeta{ Name: "console", Namespace: "default", }, - Spec: v1alpha1.ConsoleSpec{ - Server: v1alpha1.Server{ + Spec: vectorizedv1alpha1.ConsoleSpec{ + Server: vectorizedv1alpha1.Server{ HTTPListenPort: 8080, }, - ClusterRef: v1alpha1.NamespaceNameRef{ + ClusterRef: vectorizedv1alpha1.NamespaceNameRef{ Name: "cluster", Namespace: "default", }, - Deployment: v1alpha1.Deployment{ + Deployment: vectorizedv1alpha1.Deployment{ Image: "vectorized/console:master-173596f", }, - Connect: v1alpha1.Connect{Enabled: true}, - Cloud: &v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + Connect: vectorizedv1alpha1.Connect{Enabled: true}, + Cloud: &vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "test", - PasswordRef: v1alpha1.SecretKeyRef{ + PasswordRef: vectorizedv1alpha1.SecretKeyRef{ Name: "prom-pass", Namespace: "default", Key: "pass", }, }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "test", - Jobs: []v1alpha1.PrometheusScraperJobConfig{ + Jobs: []vectorizedv1alpha1.PrometheusScraperJobConfig{ {JobName: "test", KeepLabels: []string{"test"}}, }, }, @@ -225,10 +253,10 @@ func TestDoNotValidateWhenDeleted(t *testing.T) { err = c.Create(ctx, &cluster) require.NoError(t, err) - cluster.Status = v1alpha1.ClusterStatus{ - Conditions: []v1alpha1.ClusterCondition{ + cluster.Status = vectorizedv1alpha1.ClusterStatus{ + Conditions: []vectorizedv1alpha1.ClusterCondition{ { - Type: v1alpha1.ClusterConfiguredConditionType, + Type: vectorizedv1alpha1.ClusterConfiguredConditionType, Status: corev1.ConditionTrue, LastTransitionTime: metav1.Now(), }, @@ -300,23 +328,23 @@ func TestValidatePrometheus(t *testing.T) { tests := []struct { name string - cloudConfig v1alpha1.CloudConfig + cloudConfig vectorizedv1alpha1.CloudConfig expectedValidationError bool }{ - {"valid", v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + {"valid", vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "username", - PasswordRef: v1alpha1.SecretKeyRef{ + PasswordRef: vectorizedv1alpha1.SecretKeyRef{ Name: secretName, Namespace: consoleNamespace, Key: passwordKey, }, }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "address", - Jobs: []v1alpha1.PrometheusScraperJobConfig{ + Jobs: []vectorizedv1alpha1.PrometheusScraperJobConfig{ { JobName: "job", KeepLabels: []string{"label"}, @@ -325,31 +353,31 @@ func TestValidatePrometheus(t *testing.T) { }, }, }, false}, - {"missing job config", v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + {"missing job config", vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "username", - PasswordRef: v1alpha1.SecretKeyRef{ + PasswordRef: vectorizedv1alpha1.SecretKeyRef{ Name: secretName, Namespace: consoleNamespace, Key: passwordKey, }, }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "address", }, }, }, true}, - {"missing basic auth password", v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + {"missing basic auth password", vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "username", }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "address", - Jobs: []v1alpha1.PrometheusScraperJobConfig{ + Jobs: []vectorizedv1alpha1.PrometheusScraperJobConfig{ { JobName: "job", KeepLabels: []string{"label"}, @@ -358,20 +386,20 @@ func TestValidatePrometheus(t *testing.T) { }, }, }, true}, - {"nonexistent secret", v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + {"nonexistent secret", vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "username", - PasswordRef: v1alpha1.SecretKeyRef{ + PasswordRef: vectorizedv1alpha1.SecretKeyRef{ Name: "nonexisting", Namespace: consoleNamespace, Key: passwordKey, }, }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "address", - Jobs: []v1alpha1.PrometheusScraperJobConfig{ + Jobs: []vectorizedv1alpha1.PrometheusScraperJobConfig{ { JobName: "job", KeepLabels: []string{"label"}, @@ -380,20 +408,20 @@ func TestValidatePrometheus(t *testing.T) { }, }, }, true}, - {"wrong basic auth secret key", v1alpha1.CloudConfig{ - PrometheusEndpoint: &v1alpha1.PrometheusEndpointConfig{ + {"wrong basic auth secret key", vectorizedv1alpha1.CloudConfig{ + PrometheusEndpoint: &vectorizedv1alpha1.PrometheusEndpointConfig{ Enabled: true, - BasicAuth: v1alpha1.BasicAuthConfig{ + BasicAuth: vectorizedv1alpha1.BasicAuthConfig{ Username: "username", - PasswordRef: v1alpha1.SecretKeyRef{ + PasswordRef: vectorizedv1alpha1.SecretKeyRef{ Name: secretName, Namespace: consoleNamespace, Key: "nonexistingkey", }, }, - Prometheus: &v1alpha1.PrometheusConfig{ + Prometheus: &vectorizedv1alpha1.PrometheusConfig{ Address: "address", - Jobs: []v1alpha1.PrometheusScraperJobConfig{ + Jobs: []vectorizedv1alpha1.PrometheusScraperJobConfig{ { JobName: "job", KeepLabels: []string{"label"}, @@ -406,12 +434,12 @@ func TestValidatePrometheus(t *testing.T) { for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs, err := redpanda.ValidatePrometheus(context.TODO(), ctl, &v1alpha1.Console{ + errs, err := redpanda.ValidatePrometheus(context.TODO(), ctl, &vectorizedv1alpha1.Console{ ObjectMeta: metav1.ObjectMeta{ Name: "console", Namespace: consoleNamespace, }, - Spec: v1alpha1.ConsoleSpec{ + Spec: vectorizedv1alpha1.ConsoleSpec{ Cloud: &tests[i].cloudConfig, }, })