From 8618d81de6ed2c02252817cbdbe77bb686ad24ec Mon Sep 17 00:00:00 2001 From: zoumo Date: Tue, 24 Dec 2024 10:29:52 +0800 Subject: [PATCH] refactor: separate cert operations and webhook cert manager (#56) --- {webhook/cert => cert}/cert.go | 14 ++- {webhook/cert => cert}/cert_test.go | 0 {webhook/cert => cert}/error.go | 2 + {webhook/cert => cert}/error_test.go | 0 {webhook/cert => cert}/fs.go | 43 +++++---- cert/fs_test.go | 45 +++++++++ {webhook/cert => cert}/secret.go | 73 ++++++++++++--- cert/secret_test.go | 79 ++++++++++++++++ go.mod | 4 +- go.sum | 5 +- .../signer.go => certmanager/manager.go} | 91 ++++++------------- .../manager_test.go} | 16 ++-- 12 files changed, 259 insertions(+), 113 deletions(-) rename {webhook/cert => cert}/cert.go (85%) rename {webhook/cert => cert}/cert_test.go (100%) rename {webhook/cert => cert}/error.go (90%) rename {webhook/cert => cert}/error_test.go (100%) rename {webhook/cert => cert}/fs.go (83%) create mode 100644 cert/fs_test.go rename {webhook/cert => cert}/secret.go (65%) create mode 100644 cert/secret_test.go rename webhook/{cert/signer.go => certmanager/manager.go} (77%) rename webhook/{cert/signer_test.go => certmanager/manager_test.go} (95%) diff --git a/webhook/cert/cert.go b/cert/cert.go similarity index 85% rename from webhook/cert/cert.go rename to cert/cert.go index 4ed17f0..3310992 100644 --- a/webhook/cert/cert.go +++ b/cert/cert.go @@ -33,6 +33,7 @@ type ( AltNames = cert.AltNames ) +// ServingCerts is a set of serving certificates. type ServingCerts struct { Key []byte Cert []byte @@ -40,6 +41,7 @@ type ServingCerts struct { CACert []byte } +// Validate checks if the serving certificates are valid for given host. func (c *ServingCerts) Validate(host string) error { if len(c.Key) == 0 { return fmt.Errorf("private key is empty") @@ -75,6 +77,7 @@ func (c *ServingCerts) Validate(host string) error { return err } +// GenerateSelfSignedCerts generates a self-signed certificate and key for the given host. func GenerateSelfSignedCerts(cfg Config) (*ServingCerts, error) { caKey, caCert, key, cert, err := generateSelfSignedCertKey(cfg) if err != nil { @@ -94,15 +97,22 @@ func GenerateSelfSignedCerts(cfg Config) (*ServingCerts, error) { }, nil } +// GenerateSelfSignedCertKeyIfNotExist generates a self-signed certificate and +// write them to the given path if not exist. func GenerateSelfSignedCertKeyIfNotExist(path string, cfg cert.Config) error { - fscerts, err := NewFSProvider(path, FSOptions{}) + fscerts, err := NewFSCertProvider(path, FSOptions{}) if err != nil { return err } - return fscerts.Ensure(context.Background(), cfg) + _, err = fscerts.Ensure(context.Background(), cfg) + return err } func generateSelfSignedCertKey(cfg Config) (*rsa.PrivateKey, *x509.Certificate, *rsa.PrivateKey, *x509.Certificate, error) { + if len(cfg.CommonName) == 0 { + return nil, nil, nil, nil, fmt.Errorf("common name is empty") + } + caKey, err := certutil.NewRSAPrivateKey() if err != nil { return nil, nil, nil, nil, err diff --git a/webhook/cert/cert_test.go b/cert/cert_test.go similarity index 100% rename from webhook/cert/cert_test.go rename to cert/cert_test.go diff --git a/webhook/cert/error.go b/cert/error.go similarity index 90% rename from webhook/cert/error.go rename to cert/error.go index bfacac6..066936d 100644 --- a/webhook/cert/error.go +++ b/cert/error.go @@ -29,10 +29,12 @@ func newNotFound(name string, err error) error { return fmt.Errorf("%s %w: %v", name, errNotFound, err) } +// IsNotFound returns true if certificate not found. func IsNotFound(err error) bool { return apierrors.IsNotFound(err) || errors.Is(err, errNotFound) } +// IsConflict returns true if certificate is already exist. func IsConflict(err error) bool { return apierrors.IsAlreadyExists(err) || apierrors.IsConflict(err) } diff --git a/webhook/cert/error_test.go b/cert/error_test.go similarity index 100% rename from webhook/cert/error_test.go rename to cert/error_test.go diff --git a/webhook/cert/fs.go b/cert/fs.go similarity index 83% rename from webhook/cert/fs.go rename to cert/fs.go index 509ee85..73f625a 100644 --- a/webhook/cert/fs.go +++ b/cert/fs.go @@ -27,11 +27,7 @@ import ( "k8s.io/klog/v2" ) -type FSProvider struct { - FSOptions - path string -} - +// FSOptions is the options for FSCertProvider. type FSOptions struct { FS afero.Fs CertName string @@ -58,49 +54,56 @@ func (o *FSOptions) setDefaults() { } } -func NewFSProvider(path string, opts FSOptions) (*FSProvider, error) { +// FSCertProvider is a CertProvider that stores certificates on the local filesystem. +type FSCertProvider struct { + FSOptions + path string +} + +// NewFSCertProvider creates a new FSCertProvider. +func NewFSCertProvider(path string, opts FSOptions) (*FSCertProvider, error) { opts.setDefaults() if len(path) == 0 { return nil, fmt.Errorf("cert path is required") } - return &FSProvider{ + return &FSCertProvider{ path: path, FSOptions: opts, }, nil } -func (p *FSProvider) Ensure(_ context.Context, cfg Config) error { - certs, err := p.Load() +func (p *FSCertProvider) Ensure(ctx context.Context, cfg Config) (*ServingCerts, error) { + certs, err := p.Load(ctx) if err != nil && !IsNotFound(err) { - return err + return nil, err } if IsNotFound(err) { certs, err = GenerateSelfSignedCerts(cfg) if err != nil { - return err + return nil, err } _, err = p.Overwrite(certs) - return err + return certs, err } err = certs.Validate(cfg.CommonName) if err != nil { // re-generate if expired or invalid klog.Info("certificates are invalid, regenerating...") - certs, err := GenerateSelfSignedCerts(cfg) + certs, err = GenerateSelfSignedCerts(cfg) if err != nil { - return err + return nil, err } _, err = p.Overwrite(certs) - return err + return certs, err } - return nil + return certs, nil } -func (p *FSProvider) checkIfExist() error { +func (p *FSCertProvider) checkIfExist() error { files := []string{ path.Join(p.path, p.KeyName), path.Join(p.path, p.CertName), @@ -121,7 +124,7 @@ func (p *FSProvider) checkIfExist() error { return nil } -func (p *FSProvider) Load() (*ServingCerts, error) { +func (p *FSCertProvider) Load(_ context.Context) (*ServingCerts, error) { err := p.checkIfExist() if err != nil { return nil, err @@ -154,7 +157,7 @@ func (p *FSProvider) Load() (*ServingCerts, error) { return certs, nil } -func (p *FSProvider) Overwrite(certs *ServingCerts) (bool, error) { +func (p *FSCertProvider) Overwrite(certs *ServingCerts) (bool, error) { if certs == nil { return false, fmt.Errorf("certs are required") } @@ -206,7 +209,7 @@ func (p *FSProvider) Overwrite(certs *ServingCerts) (bool, error) { return updated, nil } -func (p *FSProvider) writeFile(path string, data []byte) (bool, error) { +func (p *FSCertProvider) writeFile(path string, data []byte) (bool, error) { _, err := p.FS.Stat(path) if err != nil && !os.IsNotExist(err) { return false, err diff --git a/cert/fs_test.go b/cert/fs_test.go new file mode 100644 index 0000000..b3495b6 --- /dev/null +++ b/cert/fs_test.go @@ -0,0 +1,45 @@ +/** + * Copyright 2024 The KusionStack Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cert + +import ( + "context" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" +) + +func TestFSProvider_Ensure(t *testing.T) { + dir := "/serving/cert" + fs := afero.NewMemMapFs() + provider, _ := NewFSCertProvider(dir, FSOptions{ + FS: fs, + }) + + domains := []string{"one.kusionstack.io", "two.kusionstack.io"} + for _, domain := range domains { + certs, err := provider.Ensure(context.Background(), Config{CommonName: domain}) + assert.NoError(t, err) + certs.Validate(domain) + assert.NotNil(t, certs) + certs, err = provider.Load(context.Background()) + assert.NoError(t, err) + certs.Validate(domain) + assert.NotNil(t, certs) + } +} diff --git a/webhook/cert/secret.go b/cert/secret.go similarity index 65% rename from webhook/cert/secret.go rename to cert/secret.go index 65cb7b7..7e8c500 100644 --- a/webhook/cert/secret.go +++ b/cert/secret.go @@ -20,9 +20,11 @@ import ( "context" "fmt" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -34,30 +36,76 @@ const ( TLSCAPrivateKeyKey = "ca.key" ) -type SecretProvider struct { - client SecretClient - namespace string - name string -} - +// SecretClient is a client wrapper for secret operations. type SecretClient interface { Get(ctx context.Context, namespace string, name string) (*corev1.Secret, error) Create(ctx context.Context, secret *corev1.Secret) error Update(ctx context.Context, secret *corev1.Secret) error } -func NewSecretProvider(client SecretClient, namespace, name string) (*SecretProvider, error) { +var _ SecretClient = &secretClient{} + +type secretClient struct { + reader client.Reader + writer client.Writer +} + +func NewSecretClient(reader client.Reader, writer client.Writer) SecretClient { + return &secretClient{ + reader: reader, + writer: writer, + } +} + +// Create implements SecretClient. +func (s *secretClient) Create(ctx context.Context, secret *corev1.Secret) error { + err := s.writer.Create(ctx, secret) + if err == nil { + logger := logr.FromContextOrDiscard(ctx) + logger.Info("create secret successfully", "namespace", secret.Namespace, "name", secret.Name) + } + return err +} + +// Get implements SecretClient. +func (s *secretClient) Get(ctx context.Context, namespace string, name string) (*corev1.Secret, error) { + var secret corev1.Secret + err := s.reader.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &secret) + if err != nil { + return nil, err + } + return &secret, nil +} + +// Update implements SecretClient. +func (s *secretClient) Update(ctx context.Context, secret *corev1.Secret) error { + err := s.writer.Update(ctx, secret) + if err == nil { + logger := logr.FromContextOrDiscard(ctx) + logger.Info("update secret successfully", "namespace", secret.Namespace, "name", secret.Name) + } + return err +} + +// SecretCertProvider is a provider for operating certs in k8s secret. +type SecretCertProvider struct { + client SecretClient + namespace string + name string +} + +func NewSecretCertProvider(client SecretClient, namespace, name string) (*SecretCertProvider, error) { if client == nil { return nil, fmt.Errorf("secret client must not be nil") } - return &SecretProvider{ + return &SecretCertProvider{ client: client, namespace: namespace, name: name, }, nil } -func (p *SecretProvider) Ensure(ctx context.Context, cfg Config) (*ServingCerts, error) { +func (p *SecretCertProvider) Ensure(ctx context.Context, cfg Config) (*ServingCerts, error) { certs, err := p.Load(ctx) if err != nil && !IsNotFound(err) { return nil, err @@ -91,16 +139,15 @@ func (p *SecretProvider) Ensure(ctx context.Context, cfg Config) (*ServingCerts, return certs, nil } -func (p *SecretProvider) Load(ctx context.Context) (*ServingCerts, error) { +func (p *SecretCertProvider) Load(ctx context.Context) (*ServingCerts, error) { secret, err := p.client.Get(ctx, p.namespace, p.name) if err != nil { return nil, err } - return convertSecretToCerts(secret), nil } -func (p *SecretProvider) create(ctx context.Context, certs *ServingCerts) error { +func (p *SecretCertProvider) create(ctx context.Context, certs *ServingCerts) error { if certs == nil { return fmt.Errorf("certs are required") } @@ -118,7 +165,7 @@ func (p *SecretProvider) create(ctx context.Context, certs *ServingCerts) error return p.client.Create(ctx, secret) } -func (p *SecretProvider) overwrite(ctx context.Context, certs *ServingCerts) error { +func (p *SecretCertProvider) overwrite(ctx context.Context, certs *ServingCerts) error { if certs == nil { return fmt.Errorf("certs are required") } diff --git a/cert/secret_test.go b/cert/secret_test.go new file mode 100644 index 0000000..11d5d40 --- /dev/null +++ b/cert/secret_test.go @@ -0,0 +1,79 @@ +/** + * Copyright 2024 The KusionStack Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cert + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestSecretProvider_Ensure(t *testing.T) { + provider, err := NewSecretCertProvider(newSecretClient(), "default", "test-serving-cert") + assert.NoError(t, err) + + domains := []string{"one.kusionstack.io", "two.kusionstack.io"} + for _, domain := range domains { + certs, err := provider.Ensure(context.Background(), Config{CommonName: domain}) + assert.NoError(t, err) + certs.Validate(domain) + assert.NotNil(t, certs) + certs, err = provider.Load(context.Background()) + assert.NoError(t, err) + certs.Validate(domain) + assert.NotNil(t, certs) + } +} + +func newSecretClient() SecretClient { + clientbuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + client := clientbuilder.Build() + + return NewSecretClient(client, client) +} + +// var _ SecretClient = &fakeSecretClient{} + +// type fakeSecretClient struct { +// client client.Client +// } + +// // Create implements SecretClient. +// // Subtle: this method shadows the method (Client).Create of fakeSecretClient.Client. +// func (f *fakeSecretClient) Create(ctx context.Context, secret *v1.Secret) error { +// return f.client.Create(ctx, secret) +// } + +// // Get implements SecretClient. +// // Subtle: this method shadows the method (Client).Get of fakeSecretClient.Client. +// func (f *fakeSecretClient) Get(ctx context.Context, namespace string, name string) (*v1.Secret, error) { +// var secret v1.Secret +// err := f.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, &secret) +// if err != nil { +// return nil, err +// } +// return &secret, nil +// } + +// // Update implements SecretClient. +// // Subtle: this method shadows the method (Client).Update of fakeSecretClient.Client. +// func (f *fakeSecretClient) Update(ctx context.Context, secret *v1.Secret) error { +// return f.client.Update(ctx, secret) +// } diff --git a/go.mod b/go.mod index 5411477..cf24ae4 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( k8s.io/client-go v0.28.4 k8s.io/klog/v2 v2.100.1 k8s.io/kubectl v0.28.4 + k8s.io/utils v0.0.0-20230726121419-3b25d923346b kusionstack.io/kube-api v0.2.0 sigs.k8s.io/controller-runtime v0.15.1 ) @@ -66,7 +67,6 @@ require ( k8s.io/apiextensions-apiserver v0.26.1 // indirect k8s.io/component-base v0.28.4 // indirect k8s.io/kube-openapi v0.0.0-20230717233707-2695361300d9 // indirect - k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect ) @@ -110,7 +110,7 @@ replace ( k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.22.2 k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.22.2 k8s.io/system-validators => k8s.io/system-validators v1.5.0 - k8s.io/utils => k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a + k8s.io/utils => k8s.io/utils v0.0.0-20240102154912-e7106e64919e kusionstack.io/kube-api => kusionstack.io/kube-api v0.2.0 sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.10.3 ) diff --git a/go.sum b/go.sum index 3c7eeda..c3ca437 100644 --- a/go.sum +++ b/go.sum @@ -388,7 +388,6 @@ github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4k github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE1GqG0= github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= -github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.5.1 h1:VHu76Lk0LSP1x254maIu2bplkWpfBWI+B+6fdoZprcg= github.com/spf13/afero v1.5.1/go.mod h1:Ai8FlHk4v/PARR026UzYexafAt9roJ7LcLMAmO6Z93I= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= @@ -822,8 +821,8 @@ k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e/go.mod h1:vHXdDvt9+2spS2R k8s.io/kubectl v0.22.2 h1:KMyYNZoBshaL3XKx04X07DtpoD4vMrdkfiN/G2Qx/PU= k8s.io/kubectl v0.22.2/go.mod h1:BApg2j0edxLArCOfO0ievI27EeTQqBDMNU9VQH734iQ= k8s.io/metrics v0.22.2/go.mod h1:GUcsBtpsqQD1tKFS/2wCKu4ZBowwRncLOJH1rgWs3uw= -k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a h1:8dYfu/Fc9Gz2rNJKB9IQRGgQOh2clmRzNIPPY1xLY5g= -k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCfRziVtos3ofG/sQ= +k8s.io/utils v0.0.0-20240102154912-e7106e64919e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= kusionstack.io/kube-api v0.2.0 h1:40SHCpm9RdabTUTVjhsHWoX+h7djy4jMYYTcbnJ9SQc= kusionstack.io/kube-api v0.2.0/go.mod h1:fYwuojoLs71ox8uyvyKNsJU4CtPtptSfJ3PUSt/3Hgg= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/webhook/cert/signer.go b/webhook/certmanager/manager.go similarity index 77% rename from webhook/cert/signer.go rename to webhook/certmanager/manager.go index 668a46b..b066e8c 100644 --- a/webhook/cert/signer.go +++ b/webhook/certmanager/manager.go @@ -14,7 +14,7 @@ * limitations under the License. */ -package cert +package certmanager import ( "context" @@ -37,17 +37,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "kusionstack.io/kube-utils/cert" "kusionstack.io/kube-utils/controller/mixin" ) -type WebhookCertSelfSigner struct { - *mixin.ReconcilerMixin - CertConfig - - fs *FSProvider - secret *SecretProvider -} - type CertConfig struct { Host string AlternateHosts []string @@ -60,29 +53,36 @@ type CertConfig struct { ContextWrapper func(context.Context) context.Context } -func New(mgr manager.Manager, cfg CertConfig) *WebhookCertSelfSigner { - return &WebhookCertSelfSigner{ - ReconcilerMixin: mixin.NewReconcilerMixin("webhook-cert-self-signer", mgr), +// WebhookServingCertManager is a controller that manages the webhook certs. +// It will generate and rotate the certs when it is invalid. +type WebhookServingCertManager struct { + *mixin.ReconcilerMixin + CertConfig + + fs *cert.FSCertProvider + secret *cert.SecretCertProvider +} + +// New returns a new WebhookServingCertManager. +func New(mgr manager.Manager, cfg CertConfig) *WebhookServingCertManager { + return &WebhookServingCertManager{ + ReconcilerMixin: mixin.NewReconcilerMixin("webhook-serving-cert-manager", mgr), CertConfig: cfg, } } -func (s *WebhookCertSelfSigner) SetupWithManager(mgr manager.Manager) error { +func (s *WebhookServingCertManager) SetupWithManager(mgr manager.Manager) error { var err error server := mgr.GetWebhookServer() - s.fs, err = NewFSProvider(server.CertDir, FSOptions{ + s.fs, err = cert.NewFSCertProvider(server.CertDir, cert.FSOptions{ CertName: server.CertName, KeyName: server.KeyName, }) if err != nil { return err } - s.secret, err = NewSecretProvider( - &secretClient{ - reader: s.APIReader, - writer: s.Client, - logger: s.Logger, - }, + s.secret, err = cert.NewSecretCertProvider( + cert.NewSecretClient(s.APIReader, s.Client), s.Namespace, s.SecretName, ) @@ -125,7 +125,7 @@ func (s *WebhookCertSelfSigner) SetupWithManager(mgr manager.Manager) error { return mgr.Add(&nonLeaderElectionController{Controller: ctrl}) } -func (s *WebhookCertSelfSigner) predictFunc() predicate.Funcs { +func (s *WebhookServingCertManager) predictFunc() predicate.Funcs { mutatingWebhookNameSet := sets.NewString(s.MutatingWebhookNames...) validatingWebhookNameSet := sets.NewString(s.ValidatingWebhookNames...) return predicate.NewPredicateFuncs(func(obj client.Object) bool { @@ -144,7 +144,7 @@ func (s *WebhookCertSelfSigner) predictFunc() predicate.Funcs { }) } -func (s *WebhookCertSelfSigner) enqueueSecret() handler.EventHandler { +func (s *WebhookServingCertManager) enqueueSecret() handler.EventHandler { mapFunc := func(obj client.Object) []reconcile.Request { return []reconcile.Request{ { @@ -158,19 +158,20 @@ func (s *WebhookCertSelfSigner) enqueueSecret() handler.EventHandler { return handler.EnqueueRequestsFromMapFunc(mapFunc) } -func (s *WebhookCertSelfSigner) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) { +func (s *WebhookServingCertManager) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) { + ctx = logr.NewContext(ctx, s.Logger) if s.ContextWrapper != nil { ctx = s.ContextWrapper(ctx) } - cfg := Config{ + cfg := cert.Config{ CommonName: s.Host, - AltNames: AltNames{ + AltNames: cert.AltNames{ DNSNames: s.AlternateHosts, }, } servingCerts, err := s.secret.Ensure(ctx, cfg) if err != nil { - if IsConflict(err) { + if cert.IsConflict(err) { // create error on AlreadyExists // update error on Conflict // retry @@ -202,7 +203,7 @@ func (s *WebhookCertSelfSigner) Reconcile(ctx context.Context, _ reconcile.Reque return reconcile.Result{}, nil } -func (s *WebhookCertSelfSigner) ensureWebhookConfiguration(ctx context.Context, caBundle []byte) error { +func (s *WebhookServingCertManager) ensureWebhookConfiguration(ctx context.Context, caBundle []byte) error { var errList []error mutatingCfg := &admissionregistrationv1.MutatingWebhookConfiguration{} for _, name := range s.MutatingWebhookNames { @@ -286,39 +287,3 @@ type nonLeaderElectionController struct { func (c *nonLeaderElectionController) NeedLeaderElection() bool { return false } - -var _ SecretClient = &secretClient{} - -type secretClient struct { - reader client.Reader - writer client.Writer - logger logr.Logger -} - -// Create implements SecretClient. -func (s *secretClient) Create(ctx context.Context, secret *corev1.Secret) error { - err := s.writer.Create(ctx, secret) - if err == nil { - s.logger.Info("create secret successfully", "namespace", secret.Namespace, "name", secret.Name) - } - return err -} - -// Get implements SecretClient. -func (s *secretClient) Get(ctx context.Context, namespace string, name string) (*corev1.Secret, error) { - var secret corev1.Secret - err := s.reader.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &secret) - if err != nil { - return nil, err - } - return &secret, nil -} - -// Update implements SecretClient. -func (s *secretClient) Update(ctx context.Context, secret *corev1.Secret) error { - err := s.writer.Update(ctx, secret) - if err == nil { - s.logger.Info("update secret successfully", "namespace", secret.Namespace, "name", secret.Name) - } - return err -} diff --git a/webhook/cert/signer_test.go b/webhook/certmanager/manager_test.go similarity index 95% rename from webhook/cert/signer_test.go rename to webhook/certmanager/manager_test.go index 69b426a..9821337 100644 --- a/webhook/cert/signer_test.go +++ b/webhook/certmanager/manager_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cert +package certmanager import ( "context" @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -55,7 +56,6 @@ var ( ) var _ = Describe("Self signer", func() { - It("sign cert for webhooks reconcile", func() { none := admissionregistrationv1.SideEffectClassNone mutatingWebhook := &admissionregistrationv1.MutatingWebhookConfiguration{ @@ -66,7 +66,7 @@ var _ = Describe("Self signer", func() { { Name: "hook-1.test.io", ClientConfig: admissionregistrationv1.WebhookClientConfig{ - URL: strPointer("https://test.io"), + URL: ptr.To("https://test.io"), }, SideEffects: &none, AdmissionReviewVersions: []string{"v1beta1"}, @@ -74,7 +74,7 @@ var _ = Describe("Self signer", func() { { Name: "hook-2.test.io", ClientConfig: admissionregistrationv1.WebhookClientConfig{ - URL: strPointer("https://test.io"), + URL: ptr.To("https://test.io"), }, SideEffects: &none, AdmissionReviewVersions: []string{"v1beta1"}, @@ -90,7 +90,7 @@ var _ = Describe("Self signer", func() { { Name: "hook-1.test.io", ClientConfig: admissionregistrationv1.WebhookClientConfig{ - URL: strPointer("https://test.io"), + URL: ptr.To("https://test.io"), }, SideEffects: &none, AdmissionReviewVersions: []string{"v1beta1"}, @@ -98,7 +98,7 @@ var _ = Describe("Self signer", func() { { Name: "hook-2.test.io", ClientConfig: admissionregistrationv1.WebhookClientConfig{ - URL: strPointer("https://test.io"), + URL: ptr.To("https://test.io"), }, SideEffects: &none, AdmissionReviewVersions: []string{"v1beta1"}, @@ -203,7 +203,3 @@ func createNamespace(c client.Client, namespaceName string) error { return c.Create(context.TODO(), ns) } - -func strPointer(str string) *string { - return &str -}