diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index a96f902916..569ee0277f 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -23,17 +23,17 @@ import ( _ "net/http/pprof" "time" - "github.com/openkruise/kruise/pkg/daemon" - "sigs.k8s.io/controller-runtime/pkg/manager/signals" - - "github.com/openkruise/kruise/pkg/client" - "github.com/openkruise/kruise/pkg/features" - utilfeature "github.com/openkruise/kruise/pkg/util/feature" "github.com/spf13/pflag" "k8s.io/klog/v2" "k8s.io/klog/v2/klogr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/manager/signals" + + "github.com/openkruise/kruise/pkg/client" + "github.com/openkruise/kruise/pkg/daemon" + "github.com/openkruise/kruise/pkg/features" + utilfeature "github.com/openkruise/kruise/pkg/util/feature" ) var ( diff --git a/pkg/webhook/cloneset/mutating/cloneset_create_update_handler.go b/pkg/webhook/cloneset/mutating/cloneset_create_update_handler.go index 0c55cbc683..92ac397d3a 100644 --- a/pkg/webhook/cloneset/mutating/cloneset_create_update_handler.go +++ b/pkg/webhook/cloneset/mutating/cloneset_create_update_handler.go @@ -22,25 +22,20 @@ import ( "net/http" "reflect" + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "github.com/openkruise/kruise/apis/apps/defaults" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" "github.com/openkruise/kruise/pkg/features" "github.com/openkruise/kruise/pkg/util" utilfeature "github.com/openkruise/kruise/pkg/util/feature" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // CloneSetCreateUpdateHandler handles CloneSet type CloneSetCreateUpdateHandler struct { - // To use the client, you need to do the following: - // - uncomment it - // - import sigs.k8s.io/controller-runtime/pkg/client - // - uncomment the InjectClient method at the bottom of this file. - // Client client.Client - // Decoder decodes objects Decoder *admission.Decoder } @@ -87,14 +82,6 @@ func (h *CloneSetCreateUpdateHandler) Handle(ctx context.Context, req admission. return resp } -//var _ inject.Client = &CloneSetCreateUpdateHandler{} -// -//// InjectClient injects the client into the CloneSetCreateUpdateHandler -//func (h *CloneSetCreateUpdateHandler) InjectClient(c client.Client) error { -// h.Client = c -// return nil -//} - var _ admission.DecoderInjector = &CloneSetCreateUpdateHandler{} // InjectDecoder injects the decoder into the CloneSetCreateUpdateHandler diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 7857e53964..e3587ec8af 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -22,9 +22,6 @@ import ( "net/http" "time" - webhookutil "github.com/openkruise/kruise/pkg/webhook/util" - webhookcontroller "github.com/openkruise/kruise/pkg/webhook/util/controller" - "github.com/openkruise/kruise/pkg/webhook/util/health" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/rest" "k8s.io/klog/v2" @@ -32,6 +29,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "sigs.k8s.io/controller-runtime/pkg/webhook/conversion" + + webhookutil "github.com/openkruise/kruise/pkg/webhook/util" + webhookcontroller "github.com/openkruise/kruise/pkg/webhook/util/controller" + "github.com/openkruise/kruise/pkg/webhook/util/health" ) type GateFunc func() (enabled bool) diff --git a/pkg/webhook/util/configuration/configuration.go b/pkg/webhook/util/configuration/configuration.go index 026c897f79..58209fc1f7 100644 --- a/pkg/webhook/util/configuration/configuration.go +++ b/pkg/webhook/util/configuration/configuration.go @@ -23,13 +23,13 @@ import ( "net/url" "reflect" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - webhookutil "github.com/openkruise/kruise/pkg/webhook/util" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + webhookutil "github.com/openkruise/kruise/pkg/webhook/util" ) const ( @@ -73,10 +73,12 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]admission.Handle if wh.ClientConfig.Service != nil { wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace() wh.ClientConfig.Service.Name = webhookutil.GetServiceName() + + if host := webhookutil.GetHost(); len(host) > 0 { + convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort()) + } } - if host := webhookutil.GetHost(); len(host) > 0 && wh.ClientConfig.Service != nil { - convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort()) - } + mutatingWHs = append(mutatingWHs, *wh) } mutatingConfig.Webhooks = mutatingWHs @@ -96,10 +98,12 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]admission.Handle if wh.ClientConfig.Service != nil { wh.ClientConfig.Service.Namespace = webhookutil.GetNamespace() wh.ClientConfig.Service.Name = webhookutil.GetServiceName() + + if host := webhookutil.GetHost(); len(host) > 0 { + convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort()) + } } - if host := webhookutil.GetHost(); len(host) > 0 && wh.ClientConfig.Service != nil { - convertClientConfig(&wh.ClientConfig, host, webhookutil.GetPort()) - } + validatingWHs = append(validatingWHs, *wh) } validatingConfig.Webhooks = validatingWHs @@ -122,7 +126,8 @@ func Ensure(kubeClient clientset.Interface, handlers map[string]admission.Handle func getPath(clientConfig *admissionregistrationv1.WebhookClientConfig) (string, error) { if clientConfig.Service != nil { return *clientConfig.Service.Path, nil - } else if clientConfig.URL != nil { + } + if clientConfig.URL != nil { u, err := url.Parse(*clientConfig.URL) if err != nil { return "", err diff --git a/pkg/webhook/util/controller/webhook_controller.go b/pkg/webhook/util/controller/webhook_controller.go index 6b72e5c0cd..882844e5dd 100644 --- a/pkg/webhook/util/controller/webhook_controller.go +++ b/pkg/webhook/util/controller/webhook_controller.go @@ -22,12 +22,6 @@ import ( "sync" "time" - extclient "github.com/openkruise/kruise/pkg/client" - webhookutil "github.com/openkruise/kruise/pkg/webhook/util" - "github.com/openkruise/kruise/pkg/webhook/util/configuration" - "github.com/openkruise/kruise/pkg/webhook/util/crd" - "github.com/openkruise/kruise/pkg/webhook/util/generator" - "github.com/openkruise/kruise/pkg/webhook/util/writer" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" v1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -46,6 +40,13 @@ import ( "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + extclient "github.com/openkruise/kruise/pkg/client" + webhookutil "github.com/openkruise/kruise/pkg/webhook/util" + "github.com/openkruise/kruise/pkg/webhook/util/configuration" + "github.com/openkruise/kruise/pkg/webhook/util/crd" + "github.com/openkruise/kruise/pkg/webhook/util/generator" + "github.com/openkruise/kruise/pkg/webhook/util/writer" ) const ( @@ -72,13 +73,10 @@ type Controller struct { handlers map[string]admission.Handler informerFactory informers.SharedInformerFactory - //secretLister corelisters.SecretNamespaceLister - //mutatingWCLister admissionregistrationlisters.MutatingWebhookConfigurationLister - //validatingWCLister admissionregistrationlisters.ValidatingWebhookConfigurationLister - crdClient apiextensionsclientset.Interface - crdInformer cache.SharedIndexInformer - crdLister apiextensionslisters.CustomResourceDefinitionLister - synced []cache.InformerSynced + crdClient apiextensionsclientset.Interface + crdInformer cache.SharedIndexInformer + crdLister apiextensionslisters.CustomResourceDefinitionLister + synced []cache.InformerSynced queue workqueue.RateLimitingInterface } @@ -94,9 +92,6 @@ func New(cfg *rest.Config, handlers map[string]admission.Handler) (*Controller, secretInformer := coreinformers.New(c.informerFactory, namespace, nil).Secrets() admissionRegistrationInformer := admissionregistrationinformers.New(c.informerFactory, v1.NamespaceAll, nil) - //c.secretLister = secretInformer.Lister().Secrets(namespace) - //c.mutatingWCLister = admissionRegistrationInformer.MutatingWebhookConfigurations().Lister() - //c.validatingWCLister = admissionRegistrationInformer.ValidatingWebhookConfigurations().Lister() secretInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { @@ -229,14 +224,14 @@ func (c *Controller) sync() error { klog.Infof("Finished to sync webhook certs and configurations") }() - var dnsName string - var certWriter writer.CertWriter - var err error - - if dnsName = webhookutil.GetHost(); len(dnsName) == 0 { + dnsName := webhookutil.GetHost() + if len(dnsName) == 0 { dnsName = generator.ServiceToCommonName(webhookutil.GetNamespace(), webhookutil.GetServiceName()) } + var certWriter writer.CertWriter + var err error + certWriterType := webhookutil.GetCertWriter() if certWriterType == writer.FsCertWriter || (len(certWriterType) == 0 && len(webhookutil.GetHost()) != 0) { certWriter, err = writer.NewFSCertWriter(writer.FSCertWriterOptions{ diff --git a/pkg/webhook/util/crd/crd.go b/pkg/webhook/util/crd/crd.go index e9c311a736..d21f3ba912 100644 --- a/pkg/webhook/util/crd/crd.go +++ b/pkg/webhook/util/crd/crd.go @@ -21,8 +21,6 @@ import ( "fmt" "reflect" - "github.com/openkruise/kruise/apis" - webhookutil "github.com/openkruise/kruise/pkg/webhook/util" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apiextensionslisters "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1" @@ -30,6 +28,9 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/openkruise/kruise/apis" + webhookutil "github.com/openkruise/kruise/pkg/webhook/util" ) var ( diff --git a/pkg/webhook/util/generator/selfsigned.go b/pkg/webhook/util/generator/selfsigned.go index 704f8c5f8c..e4268d1e5b 100644 --- a/pkg/webhook/util/generator/selfsigned.go +++ b/pkg/webhook/util/generator/selfsigned.go @@ -65,12 +65,9 @@ func (cp *SelfSignedCertGenerator) SetCA(caKey, caCert []byte) { // client to verify the server authentication chain. // The cert will be valid for 365 days. func (cp *SelfSignedCertGenerator) Generate(commonName string) (*Artifacts, error) { - var signingKey *rsa.PrivateKey - var signingCert *x509.Certificate - var valid bool var err error - valid, signingKey, signingCert = cp.validCACert() + valid, signingKey, signingCert := cp.validCACert() if !valid { signingKey, err = NewPrivateKey() if err != nil { @@ -115,12 +112,10 @@ func (cp *SelfSignedCertGenerator) Generate(commonName string) (*Artifacts, erro } func (cp *SelfSignedCertGenerator) validCACert() (bool, *rsa.PrivateKey, *x509.Certificate) { - if !ValidCACert(cp.caKey, cp.caCert, cp.caCert, "", - time.Now().AddDate(1, 0, 0)) { + if !ValidCACert(cp.caKey, cp.caCert, cp.caCert, "", time.Now().AddDate(1, 0, 0)) { return false, nil, nil } - var ok bool key, err := keyutil.ParsePrivateKeyPEM(cp.caKey) if err != nil { return false, nil, nil diff --git a/pkg/webhook/util/generator/util.go b/pkg/webhook/util/generator/util.go index b2ae249545..6080a30a46 100644 --- a/pkg/webhook/util/generator/util.go +++ b/pkg/webhook/util/generator/util.go @@ -24,7 +24,7 @@ import ( "time" ) -// ValidCACert think cert and key are valid if they meet the following requirements: +// ValidCACert treats cert and key are valid if they meet the following requirements: // - key and cert are valid pair // - caCert is the root ca of cert // - cert is for dnsName diff --git a/pkg/webhook/util/health/checker.go b/pkg/webhook/util/health/checker.go index c10a0dca8f..35bde41ac7 100644 --- a/pkg/webhook/util/health/checker.go +++ b/pkg/webhook/util/health/checker.go @@ -132,8 +132,5 @@ func Checker(_ *http.Request) error { lock.Lock() defer lock.Unlock() _, err = client.Do(req) - if err != nil { - return err - } - return nil + return err } diff --git a/pkg/webhook/util/writer/certwriter.go b/pkg/webhook/util/writer/certwriter.go index 8b6a526703..7fe3271d84 100644 --- a/pkg/webhook/util/writer/certwriter.go +++ b/pkg/webhook/util/writer/certwriter.go @@ -18,14 +18,12 @@ limitations under the License. package writer import ( - "crypto/tls" - "crypto/x509" - "encoding/pem" "errors" "time" - "github.com/openkruise/kruise/pkg/webhook/util/generator" "k8s.io/klog/v2" + + "github.com/openkruise/kruise/pkg/webhook/util/generator" ) const ( @@ -65,7 +63,7 @@ func handleCommon(dnsName string, ch certReadWriter) (*generator.Artifacts, bool // Recreate the cert if it's invalid. valid := validCert(certs, dnsName) if !valid { - klog.Info("cert is invalid or expiring, regenerating a new one") + klog.Info("cert is invalid or expired, regenerating a new one") certs, err = ch.overwrite(certs.ResourceVersion) if err != nil { return nil, false, err @@ -81,57 +79,29 @@ func createIfNotExists(ch certReadWriter) (*generator.Artifacts, bool, error) { if isNotFound(err) { // Create if not exists certs, err = ch.write() - switch { // This may happen if there is another racer. - case isAlreadyExists(err): + if isAlreadyExists(err) { certs, err = ch.read() - return certs, true, err - default: - return certs, true, err } + return certs, true, err } return certs, false, err } // certReadWriter provides methods for reading and writing certificates. type certReadWriter interface { - // read reads a webhook name and returns the certs for it. + // read a webhook name and returns the certs for it. read() (*generator.Artifacts, error) - // write writes the certs and return the certs it wrote. + // write the certs and return the certs it wrote. write() (*generator.Artifacts, error) - // overwrite overwrites the existing certs and return the certs it wrote. + // overwrite the existing certs and return the certs it wrote. overwrite(resourceVersion string) (*generator.Artifacts, error) } func validCert(certs *generator.Artifacts, dnsName string) bool { - if certs == nil || certs.Cert == nil || certs.Key == nil || certs.CACert == nil { - return false - } - - // Verify key and cert are valid pair - _, err := tls.X509KeyPair(certs.Cert, certs.Key) - if err != nil { - return false - } - - // Verify cert is good for desired DNS name and signed by CA and will be valid for desired period of time. - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(certs.CACert) { - return false - } - block, _ := pem.Decode(certs.Cert) - if block == nil { + if certs == nil { return false } - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return false - } - ops := x509.VerifyOptions{ - DNSName: dnsName, - Roots: pool, - CurrentTime: time.Now().AddDate(0, 6, 0), - } - _, err = cert.Verify(ops) - return err == nil + expired := time.Now().AddDate(0, 6, 0) + return generator.ValidCACert(certs.Key, certs.Cert, certs.CACert, dnsName, expired) } diff --git a/pkg/webhook/util/writer/fs.go b/pkg/webhook/util/writer/fs.go index 681240b634..91be987611 100644 --- a/pkg/webhook/util/writer/fs.go +++ b/pkg/webhook/util/writer/fs.go @@ -24,9 +24,10 @@ import ( "os" "path" + "k8s.io/klog/v2" + "github.com/openkruise/kruise/pkg/webhook/util/generator" "github.com/openkruise/kruise/pkg/webhook/util/writer/atomic" - "k8s.io/klog/v2" ) const ( @@ -71,9 +72,7 @@ func NewFSCertWriter(ops FSCertWriterOptions) (CertWriter, error) { if err != nil { return nil, err } - return &fsCertWriter{ - FSCertWriterOptions: &ops, - }, nil + return &fsCertWriter{FSCertWriterOptions: &ops}, nil } // EnsureCert provisions certificates for a webhookClientConfig by writing the certificates in the filesystem. diff --git a/pkg/webhook/util/writer/secret.go b/pkg/webhook/util/writer/secret.go index 717aa29fbd..33fc56b4d8 100644 --- a/pkg/webhook/util/writer/secret.go +++ b/pkg/webhook/util/writer/secret.go @@ -21,13 +21,14 @@ import ( "context" "errors" - "github.com/openkruise/kruise/pkg/webhook/util/generator" 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" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" + + "github.com/openkruise/kruise/pkg/webhook/util/generator" ) const ( @@ -77,9 +78,7 @@ func NewSecretCertWriter(ops SecretCertWriterOptions) (CertWriter, error) { if err != nil { return nil, err } - return &secretCertWriter{ - SecretCertWriterOptions: &ops, - }, nil + return &secretCertWriter{SecretCertWriterOptions: &ops}, nil } // EnsureCert provisions certificates for a webhookClientConfig by writing the certificates to a k8s secret. @@ -112,8 +111,7 @@ func (s *secretCertWriter) write() (*generator.Artifacts, error) { return certs, err } -func (s *secretCertWriter) overwrite(resourceVersion string) ( - *generator.Artifacts, error) { +func (s *secretCertWriter) overwrite(resourceVersion string) (*generator.Artifacts, error) { secret, certs, err := s.buildSecret() if err != nil { return nil, err @@ -122,28 +120,24 @@ func (s *secretCertWriter) overwrite(resourceVersion string) ( secret, err = s.Clientset.CoreV1().Secrets(secret.Namespace).Update(context.TODO(), secret, metav1.UpdateOptions{}) if err != nil { klog.Infof("Cert writer update secret failed: %v", err) - return certs, err + return nil, err } klog.Infof("Cert writer update secret %s resourceVersion from %s to %s", - secret.Name, resourceVersion, secret.ResourceVersion) - return certs, err + secret.Name, resourceVersion, secret.ResourceVersion, + ) + return certs, nil } func (s *secretCertWriter) read() (*generator.Artifacts, error) { - //secret := &corev1.Secret{ - // TypeMeta: metav1.TypeMeta{ - // APIVersion: "v1", - // Kind: "Secret", - // }, - //} secret, err := s.Clientset.CoreV1().Secrets(s.Secret.Namespace).Get(context.TODO(), s.Secret.Name, metav1.GetOptions{}) if apierrors.IsNotFound(err) { return nil, notFoundError{err} - } else if err != nil { + } + if err != nil { return nil, err } certs := secretToCerts(secret) - if certs != nil && certs.CACert != nil && certs.CAKey != nil { + if certs.CACert != nil && certs.CAKey != nil { // Store the CA for next usage. s.CertGenerator.SetCA(certs.CAKey, certs.CACert) } @@ -151,16 +145,16 @@ func (s *secretCertWriter) read() (*generator.Artifacts, error) { } func secretToCerts(secret *corev1.Secret) *generator.Artifacts { - if secret.Data == nil { - return &generator.Artifacts{ResourceVersion: secret.ResourceVersion} - } - return &generator.Artifacts{ - CAKey: secret.Data[CAKeyName], - CACert: secret.Data[CACertName], - Cert: secret.Data[ServerCertName], - Key: secret.Data[ServerKeyName], + ret := &generator.Artifacts{ ResourceVersion: secret.ResourceVersion, } + if secret.Data != nil { + ret.CAKey = secret.Data[CAKeyName] + ret.CACert = secret.Data[CACertName] + ret.Cert = secret.Data[ServerCertName] + ret.Key = secret.Data[ServerKeyName] + } + return ret } func certsToSecret(certs *generator.Artifacts, sec types.NamespacedName) *corev1.Secret {