diff --git a/example/main.go b/example/main.go index 738f70083d..52508e233d 100644 --- a/example/main.go +++ b/example/main.go @@ -39,11 +39,16 @@ import ( var log = logf.Log.WithName("example-controller") func main() { + var installWebhookConfig bool + flag.BoolVar(&installWebhookConfig, "install-webhook-config", false, + "enable the installer in the webhook server, so it will install webhook related resources during bootstrapping") + flag.Parse() logf.SetLogger(logf.ZapLogger(false)) entryLog := log.WithName("entrypoint") // Setup a Manager + entryLog.Info("setting up manager") mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{}) if err != nil { entryLog.Error(err, "unable to set up overall controller manager") @@ -51,6 +56,7 @@ func main() { } // Setup a new controller to Reconciler ReplicaSets + entryLog.Info("Setting up controller") c, err := controller.New("foo-controller", mgr, controller.Options{ Reconciler: &reconcileReplicaSet{client: mgr.GetClient(), log: log.WithName("reconciler")}, }) @@ -73,6 +79,7 @@ func main() { } // Setup webhooks + entryLog.Info("setting up webhooks") mutatingWebhook, err := builder.NewWebhookBuilder(). Name("mutating.k8s.io"). Mutating(). @@ -99,9 +106,11 @@ func main() { os.Exit(1) } + entryLog.Info("setting up webhook server") as, err := webhook.NewServer("foo-admission-server", mgr, webhook.ServerOptions{ - Port: 9876, - CertDir: "/tmp/cert", + Port: 9876, + CertDir: "/tmp/cert", + InstallWebhookConfig: installWebhookConfig, BootstrapOptions: &webhook.BootstrapOptions{ Secret: &apitypes.NamespacedName{ Namespace: "default", @@ -122,12 +131,15 @@ func main() { entryLog.Error(err, "unable to create a new webhook server") os.Exit(1) } + + entryLog.Info("registering webhooks to the webhook server") err = as.Register(mutatingWebhook, validatingWebhook) if err != nil { entryLog.Error(err, "unable to register webhooks in the admission server") os.Exit(1) } + entryLog.Info("starting manager") if err := mgr.Start(signals.SetupSignalHandler()); err != nil { entryLog.Error(err, "unable to run manager") os.Exit(1) diff --git a/pkg/webhook/bootstrap.go b/pkg/webhook/bootstrap.go index e4056519a7..7f93de5be9 100644 --- a/pkg/webhook/bootstrap.go +++ b/pkg/webhook/bootstrap.go @@ -22,12 +22,9 @@ import ( "net" "net/http" "net/url" - "os" "path" "strconv" - "github.com/ghodss/yaml" - "k8s.io/api/admissionregistration/v1beta1" admissionregistration "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" @@ -117,15 +114,11 @@ func (s *Server) setBootstrappingDefault() { s.certProvisioner = &cert.Provisioner{ CertWriter: certWriter, } - if s.Writer == nil { - s.Writer = os.Stdout - } } -// installWebhookConfig writes the configuration of admissionWebhookConfiguration in yaml format if dryrun is true. -// Otherwise, it creates the the admissionWebhookConfiguration objects and service if any. +// InstallWebhookManifests creates the admissionWebhookConfiguration objects and service if any. // It also provisions the certificate for the admission server. -func (s *Server) installWebhookConfig() error { +func (s *Server) InstallWebhookManifests() error { // do defaulting if necessary s.once.Do(s.setDefault) if s.err != nil { @@ -148,40 +141,14 @@ func (s *Server) installWebhookConfig() error { _, err = s.certProvisioner.Provision(cert.Options{ ClientConfig: cc, Objects: s.webhookConfigurations, - Dryrun: s.Dryrun, }) if err != nil { return err } - if s.Dryrun { - // TODO: print here - // if dryrun, return the AdmissionWebhookConfiguration in yaml format. - return s.genYamlConfig(objects) - } - return batchCreateOrReplace(s.Client, objects...) } -// genYamlConfig generates yaml config for admissionWebhookConfiguration -func (s *Server) genYamlConfig(objs []runtime.Object) error { - for _, obj := range objs { - _, err := s.Writer.Write([]byte("---")) - if err != nil { - return err - } - b, err := yaml.Marshal(obj) - if err != nil { - return err - } - _, err = s.Writer.Write(b) - if err != nil { - return err - } - } - return nil -} - func (s *Server) getClientConfig() (*admissionregistration.WebhookClientConfig, error) { if s.Host != nil && s.Service != nil { return nil, errors.New("URL and Service can't be set at the same time") diff --git a/pkg/webhook/internal/cert/generator/certgenerator.go b/pkg/webhook/internal/cert/generator/certgenerator.go index af43d1b4bc..633eefc73c 100644 --- a/pkg/webhook/internal/cert/generator/certgenerator.go +++ b/pkg/webhook/internal/cert/generator/certgenerator.go @@ -23,6 +23,8 @@ type Artifacts struct { Key []byte // PEM encoded serving certificate Cert []byte + // PEM encoded CA private key + CAKey []byte // PEM encoded CA certificate CACert []byte } @@ -31,4 +33,6 @@ type Artifacts struct { type CertGenerator interface { // Generate returns a Artifacts struct. Generate(CommonName string) (*Artifacts, error) + // SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. + SetCA(caKey, caCert []byte) } diff --git a/pkg/webhook/internal/cert/generator/fake/certgenerator.go b/pkg/webhook/internal/cert/generator/fake/certgenerator.go index d631dd216d..473f052fb4 100644 --- a/pkg/webhook/internal/cert/generator/fake/certgenerator.go +++ b/pkg/webhook/internal/cert/generator/fake/certgenerator.go @@ -17,6 +17,7 @@ limitations under the License. package fake import ( + "bytes" "fmt" "sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator" @@ -24,16 +25,29 @@ import ( // CertGenerator is a certGenerator for testing. type CertGenerator struct { + CAKey []byte + CACert []byte DNSNameToCertArtifacts map[string]*generator.Artifacts } var _ generator.CertGenerator = &CertGenerator{} +// SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. +func (cp *CertGenerator) SetCA(CAKey, CACert []byte) { + cp.CAKey = CAKey + cp.CACert = CACert +} + // Generate generates certificates by matching a common name. func (cp *CertGenerator) Generate(commonName string) (*generator.Artifacts, error) { certs, found := cp.DNSNameToCertArtifacts[commonName] if !found { return nil, fmt.Errorf("failed to find common name %q in the certGenerator", commonName) } + if cp.CAKey != nil && cp.CACert != nil && + !bytes.Contains(cp.CAKey, []byte("invalid")) && !bytes.Contains(cp.CACert, []byte("invalid")) { + certs.CAKey = cp.CAKey + certs.CACert = cp.CACert + } return certs, nil } diff --git a/pkg/webhook/internal/cert/generator/selfsigned.go b/pkg/webhook/internal/cert/generator/selfsigned.go index 0d190127ed..733b674ef7 100644 --- a/pkg/webhook/internal/cert/generator/selfsigned.go +++ b/pkg/webhook/internal/cert/generator/selfsigned.go @@ -17,8 +17,10 @@ limitations under the License. package generator import ( + "crypto/rsa" "crypto/x509" "fmt" + "time" "k8s.io/client-go/util/cert" ) @@ -30,24 +32,42 @@ func ServiceToCommonName(serviceNamespace, serviceName string) string { // SelfSignedCertGenerator implements the certGenerator interface. // It provisions self-signed certificates. -type SelfSignedCertGenerator struct{} +type SelfSignedCertGenerator struct { + caKey []byte + caCert []byte +} var _ CertGenerator = &SelfSignedCertGenerator{} +// SetCA sets the PEM-encoded CA private key and CA cert for signing the generated serving cert. +func (cp *SelfSignedCertGenerator) SetCA(caKey, caCert []byte) { + cp.caKey = caKey + cp.caCert = caCert +} + // Generate creates and returns a CA certificate, certificate and // key for the server. serverKey and serverCert are used by the server // to establish trust for clients, CA certificate is used by the // client to verify the server authentication chain. // The cert will be valid for 365 days. func (cp *SelfSignedCertGenerator) Generate(commonName string) (*Artifacts, error) { - signingKey, err := cert.NewPrivateKey() - if err != nil { - return nil, fmt.Errorf("failed to create the CA private key: %v", err) - } - signingCert, err := cert.NewSelfSignedCACert(cert.Config{CommonName: "webhook-cert-ca"}, signingKey) - if err != nil { - return nil, fmt.Errorf("failed to create the CA cert: %v", err) + var signingKey *rsa.PrivateKey + var signingCert *x509.Certificate + var valid bool + var err error + + valid, signingKey, signingCert = cp.validCACert() + if !valid { + signingKey, err = cert.NewPrivateKey() + if err != nil { + return nil, fmt.Errorf("failed to create the CA private key: %v", err) + } + signingCert, err = cert.NewSelfSignedCACert(cert.Config{CommonName: "webhook-cert-ca"}, signingKey) + if err != nil { + return nil, fmt.Errorf("failed to create the CA cert: %v", err) + } } + key, err := cert.NewPrivateKey() if err != nil { return nil, fmt.Errorf("failed to create the private key: %v", err) @@ -65,6 +85,33 @@ func (cp *SelfSignedCertGenerator) Generate(commonName string) (*Artifacts, erro return &Artifacts{ Key: cert.EncodePrivateKeyPEM(key), Cert: cert.EncodeCertPEM(signedCert), + CAKey: cert.EncodePrivateKeyPEM(signingKey), CACert: cert.EncodeCertPEM(signingCert), }, nil } + +func (cp *SelfSignedCertGenerator) validCACert() (bool, *rsa.PrivateKey, *x509.Certificate) { + if !ValidCACert(cp.caKey, cp.caCert, cp.caCert, "", + time.Now().AddDate(1, 0, 0)) { + return false, nil, nil + } + + var ok bool + key, err := cert.ParsePrivateKeyPEM(cp.caKey) + if err != nil { + return false, nil, nil + } + privateKey, ok := key.(*rsa.PrivateKey) + if !ok { + return false, nil, nil + } + + certs, err := cert.ParseCertsPEM(cp.caCert) + if err != nil { + return false, nil, nil + } + if len(certs) != 1 { + return false, nil, nil + } + return true, privateKey, certs[0] +} diff --git a/pkg/webhook/internal/cert/generator/selfsigned_test.go b/pkg/webhook/internal/cert/generator/selfsigned_test.go index 6b1a198f5c..d12e4d88e4 100644 --- a/pkg/webhook/internal/cert/generator/selfsigned_test.go +++ b/pkg/webhook/internal/cert/generator/selfsigned_test.go @@ -19,38 +19,116 @@ package generator import ( "crypto/x509" "encoding/pem" - "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" ) -func TestProvisionServingCert(t *testing.T) { +var _ = Describe("Cert Generator", func() { cn := "mysvc.myns.svc" - cp := SelfSignedCertGenerator{} - certs, _ := cp.Generate(cn) - - // First, create the set of root certificates. For this example we only - // have one. It's also possible to omit this in order to use the - // default root set of the current operating system. - roots := x509.NewCertPool() - ok := roots.AppendCertsFromPEM(certs.CACert) - if !ok { - t.Fatalf("failed to parse root certificate: %s", certs.CACert) - } - - block, _ := pem.Decode(certs.Cert) - if block == nil { - t.Fatalf("failed to parse certificate PEM: %s", certs.Cert) - } - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - t.Fatalf("failed to parse certificate: %v", err) - } - - opts := x509.VerifyOptions{ - DNSName: cn, - Roots: roots, - } - - if _, err := cert.Verify(opts); err != nil { - t.Fatalf("failed to verify certificate: %v", err) - } -} + Describe("CA doesn't exist", func() { + It("should generate CA", func() { + cp := SelfSignedCertGenerator{} + certs, err := cp.Generate(cn) + Expect(err).NotTo(HaveOccurred()) + + // First, create the set of root certificates. For this example we only + // have one. It's also possible to omit this in order to use the + // default root set of the current operating system. + roots := x509.NewCertPool() + ok := roots.AppendCertsFromPEM(certs.CACert) + Expect(ok).To(BeTrue()) + + block, _ := pem.Decode(certs.Cert) + Expect(block).NotTo(BeNil()) + + cert, err := x509.ParseCertificate(block.Bytes) + Expect(err).NotTo(HaveOccurred()) + + opts := x509.VerifyOptions{ + DNSName: cn, + Roots: roots, + } + + _, err = cert.Verify(opts) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Describe("CA doesn't exist", func() { + Context("CA is valid", func() { + It("should reuse existing CA", func() { + cp := SelfSignedCertGenerator{} + certs, err := cp.Generate("foo.example.com") + Expect(err).NotTo(HaveOccurred()) + + cp = SelfSignedCertGenerator{} + cp.SetCA(certs.CAKey, certs.CACert) + certs, err = cp.Generate(cn) + Expect(err).NotTo(HaveOccurred()) + + Expect(certs.CAKey).To(Equal(cp.caKey)) + Expect(certs.CACert).To(Equal(cp.caCert)) + + // First, create the set of root certificates. For this example we only + // have one. It's also possible to omit this in order to use the + // default root set of the current operating system. + roots := x509.NewCertPool() + ok := roots.AppendCertsFromPEM(certs.CACert) + Expect(ok).To(BeTrue()) + + block, _ := pem.Decode(certs.Cert) + Expect(block).NotTo(BeNil()) + + cert, err := x509.ParseCertificate(block.Bytes) + Expect(err).NotTo(HaveOccurred()) + + opts := x509.VerifyOptions{ + DNSName: cn, + Roots: roots, + } + + _, err = cert.Verify(opts) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Context("CA is invalid", func() { + It("should reuse existing CA", func() { + cp := SelfSignedCertGenerator{} + certs, err := cp.Generate("foo.example.com") + Expect(err).NotTo(HaveOccurred()) + + cp = SelfSignedCertGenerator{} + cp.SetCA([]byte("invalidCAKey"), []byte("invalidCACert")) + + certs, err = cp.Generate(cn) + Expect(err).NotTo(HaveOccurred()) + + Expect(certs.CAKey).NotTo(Equal(cp.caKey)) + Expect(certs.CACert).NotTo(Equal(cp.caCert)) + + // First, create the set of root certificates. For this example we only + // have one. It's also possible to omit this in order to use the + // default root set of the current operating system. + roots := x509.NewCertPool() + ok := roots.AppendCertsFromPEM(certs.CACert) + Expect(ok).To(BeTrue()) + + block, _ := pem.Decode(certs.Cert) + Expect(block).NotTo(BeNil()) + + cert, err := x509.ParseCertificate(block.Bytes) + Expect(err).NotTo(HaveOccurred()) + + opts := x509.VerifyOptions{ + DNSName: cn, + Roots: roots, + } + + _, err = cert.Verify(opts) + Expect(err).NotTo(HaveOccurred()) + }) + }) + }) +}) diff --git a/pkg/webhook/internal/cert/generator/suite_test.go b/pkg/webhook/internal/cert/generator/suite_test.go new file mode 100644 index 0000000000..89af8a8562 --- /dev/null +++ b/pkg/webhook/internal/cert/generator/suite_test.go @@ -0,0 +1,37 @@ +/* +Copyright 2018 The Kubernetes 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 + + http://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 generator + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" +) + +func TestSource(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecsWithDefaultAndCustomReporters(t, "Cert Generator Test Suite", []Reporter{envtest.NewlineReporter{}}) +} + +var _ = BeforeSuite(func(done Done) { + logf.SetLogger(logf.ZapLoggerTo(GinkgoWriter, true)) + close(done) +}, 60) diff --git a/pkg/webhook/internal/cert/generator/util.go b/pkg/webhook/internal/cert/generator/util.go new file mode 100644 index 0000000000..fd459cf05b --- /dev/null +++ b/pkg/webhook/internal/cert/generator/util.go @@ -0,0 +1,61 @@ +/* +Copyright 2018 The Kubernetes 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 + + http://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 generator + +import ( + "crypto/tls" + "crypto/x509" + "encoding/pem" + "time" +) + +// ValidCACert think 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 +// - cert won't expire before time +func ValidCACert(key, cert, caCert []byte, dnsName string, time time.Time) bool { + if len(key) == 0 || len(cert) == 0 || len(caCert) == 0 { + return false + } + // Verify key and cert are valid pair + _, err := tls.X509KeyPair(cert, key) + if err != nil { + return false + } + + // Verify cert is valid for at least 1 year. + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(caCert) { + return false + } + block, _ := pem.Decode([]byte(cert)) + if block == nil { + return false + } + c, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return false + } + ops := x509.VerifyOptions{ + DNSName: dnsName, + Roots: pool, + CurrentTime: time, + } + _, err = c.Verify(ops) + return err == nil +} diff --git a/pkg/webhook/internal/cert/provisioner.go b/pkg/webhook/internal/cert/provisioner.go index 2383038cea..01508c3fe5 100644 --- a/pkg/webhook/internal/cert/provisioner.go +++ b/pkg/webhook/internal/cert/provisioner.go @@ -46,8 +46,6 @@ type Options struct { ClientConfig *admissionregistrationv1beta1.WebhookClientConfig // Objects are the objects that will use the ClientConfig above. Objects []runtime.Object - // Dryrun controls if the objects are sent to the API server or write to io.Writer - Dryrun bool } // Provision provisions certificates for for the WebhookClientConfig. @@ -68,7 +66,7 @@ func (cp *Provisioner) Provision(options Options) (bool, error) { return false, err } - certs, changed, err := cp.CertWriter.EnsureCert(dnsName, options.Dryrun) + certs, changed, err := cp.CertWriter.EnsureCert(dnsName) if err != nil { return false, err } diff --git a/pkg/webhook/internal/cert/provisioner_test.go b/pkg/webhook/internal/cert/provisioner_test.go index b99fdc6798..4904cf6bc0 100644 --- a/pkg/webhook/internal/cert/provisioner_test.go +++ b/pkg/webhook/internal/cert/provisioner_test.go @@ -211,7 +211,7 @@ type fakeCertWriter struct { var _ writer.CertWriter = &fakeCertWriter{} -func (f *fakeCertWriter) EnsureCert(dnsName string, dryrun bool) (*generator.Artifacts, bool, error) { +func (f *fakeCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { f.invokedEnsureCert = true return &generator.Artifacts{}, true, nil } diff --git a/pkg/webhook/internal/cert/writer/certwriter.go b/pkg/webhook/internal/cert/writer/certwriter.go index 63d0d5e627..d04b07c33f 100644 --- a/pkg/webhook/internal/cert/writer/certwriter.go +++ b/pkg/webhook/internal/cert/writer/certwriter.go @@ -28,6 +28,8 @@ import ( ) const ( + // CAKeyName is the name of the CA private key + CAKeyName = "ca-key.pem" // CACertName is the name of the CA certificate CACertName = "ca-cert.pem" // ServerKeyName is the name of the server private key @@ -39,7 +41,7 @@ const ( // CertWriter provides method to handle webhooks. type CertWriter interface { // EnsureCert provisions the cert for the webhookClientConfig. - EnsureCert(dnsName string, dryrun bool) (*generator.Artifacts, bool, error) + EnsureCert(dnsName string) (*generator.Artifacts, bool, error) // Inject injects the necessary information given the objects. // It supports MutatingWebhookConfiguration and ValidatingWebhookConfiguration. Inject(objs ...runtime.Object) error diff --git a/pkg/webhook/internal/cert/writer/certwriter_test.go b/pkg/webhook/internal/cert/writer/certwriter_test.go index d517bed741..bfb2a65fad 100644 --- a/pkg/webhook/internal/cert/writer/certwriter_test.go +++ b/pkg/webhook/internal/cert/writer/certwriter_test.go @@ -32,9 +32,10 @@ var certs1, certs2 *generator.Artifacts func init() { cn1 := "example.com" cn2 := "test-service.test-svc-namespace.svc" - cp := generator.SelfSignedCertGenerator{} - certs1, _ = cp.Generate(cn1) - certs2, _ = cp.Generate(cn2) + cp1 := generator.SelfSignedCertGenerator{} + cp2 := generator.SelfSignedCertGenerator{} + certs1, _ = cp1.Generate(cn1) + certs2, _ = cp2.Generate(cn2) } type fakeCertReadWriter struct { diff --git a/pkg/webhook/internal/cert/writer/fs.go b/pkg/webhook/internal/cert/writer/fs.go index 072d8d6a8f..7508325bb1 100644 --- a/pkg/webhook/internal/cert/writer/fs.go +++ b/pkg/webhook/internal/cert/writer/fs.go @@ -72,8 +72,7 @@ func NewFSCertWriter(ops FSCertWriterOptions) (CertWriter, error) { } // EnsureCert provisions certificates for a webhookClientConfig by writing the certificates in the filesystem. -// fsCertWriter doesn't support dryrun. -func (f *fsCertWriter) EnsureCert(dnsName string, _ bool) (*generator.Artifacts, bool, error) { +func (f *fsCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { // create or refresh cert and write it to fs f.dnsName = dnsName return handleCommon(f.dnsName, f) @@ -125,7 +124,7 @@ func prepareToWrite(dir string) error { return err } - filenames := []string{CACertName, ServerCertName, ServerKeyName} + filenames := []string{CAKeyName, CACertName, ServerCertName, ServerKeyName} for _, f := range filenames { abspath := path.Join(dir, f) _, err := os.Stat(abspath) @@ -150,7 +149,11 @@ func (f *fsCertWriter) read() (*generator.Artifacts, error) { if err := ensureExist(f.Path); err != nil { return nil, err } - caBytes, err := ioutil.ReadFile(path.Join(f.Path, CACertName)) + caKeyBytes, err := ioutil.ReadFile(path.Join(f.Path, CAKeyName)) + if err != nil { + return nil, err + } + caCertBytes, err := ioutil.ReadFile(path.Join(f.Path, CACertName)) if err != nil { return nil, err } @@ -163,14 +166,15 @@ func (f *fsCertWriter) read() (*generator.Artifacts, error) { return nil, err } return &generator.Artifacts{ - CACert: caBytes, + CAKey: caKeyBytes, + CACert: caCertBytes, Cert: certBytes, Key: keyBytes, }, nil } func ensureExist(dir string) error { - filenames := []string{CACertName, ServerCertName, ServerKeyName} + filenames := []string{CAKeyName, CACertName, ServerCertName, ServerKeyName} for _, filename := range filenames { _, err := os.Stat(path.Join(dir, filename)) switch { @@ -188,6 +192,10 @@ func ensureExist(dir string) error { func certToProjectionMap(cert *generator.Artifacts) map[string]atomic.FileProjection { // TODO: figure out if we can reduce the permission. (Now it's 0666) return map[string]atomic.FileProjection{ + CAKeyName: { + Data: cert.CAKey, + Mode: 0666, + }, CACertName: { Data: cert.CACert, Mode: 0666, diff --git a/pkg/webhook/internal/cert/writer/fs_test.go b/pkg/webhook/internal/cert/writer/fs_test.go index 57f72f8216..4b39eb1246 100644 --- a/pkg/webhook/internal/cert/writer/fs_test.go +++ b/pkg/webhook/internal/cert/writer/fs_test.go @@ -60,7 +60,7 @@ var _ = Describe("fsCertWriter", func() { Context("Failed to EnsureCert", func() { Describe("empty DNS name", func() { It("should return error", func() { - _, _, err := certWriter.EnsureCert("", false) + _, _, err := certWriter.EnsureCert("") Expect(err).To(MatchError("dnsName should not be empty")) }) }) @@ -69,7 +69,7 @@ var _ = Describe("fsCertWriter", func() { Context("Succeeded to EnsureCert", func() { Context("CertGenerator is not set", func() { It("should default it and return no error", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) }) @@ -77,7 +77,7 @@ var _ = Describe("fsCertWriter", func() { Context("no existing certificate files", func() { It("should create new certificate files", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) @@ -102,7 +102,7 @@ var _ = Describe("fsCertWriter", func() { }) It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) @@ -131,7 +131,7 @@ var _ = Describe("fsCertWriter", func() { }) It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) @@ -156,7 +156,7 @@ var _ = Describe("fsCertWriter", func() { }) It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) @@ -191,7 +191,7 @@ var _ = Describe("fsCertWriter", func() { }) It("should replace with new certs", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) @@ -220,7 +220,7 @@ var _ = Describe("fsCertWriter", func() { close(done) }) It("should keep the secret", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) caBytes, err := ioutil.ReadFile(path.Join(testingDir, CACertName)) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/webhook/internal/cert/writer/secret.go b/pkg/webhook/internal/cert/writer/secret.go index d6c9e368ac..04d7e90c7d 100644 --- a/pkg/webhook/internal/cert/writer/secret.go +++ b/pkg/webhook/internal/cert/writer/secret.go @@ -18,10 +18,6 @@ package writer import ( "errors" - "io" - "os" - - "github.com/ghodss/yaml" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -38,8 +34,6 @@ type secretCertWriter struct { // dnsName is the DNS name that the certificate is for. dnsName string - // dryrun indicates sending the create/update request to the server or output to the writer in yaml format. - dryrun bool } // SecretCertWriterOptions is options for constructing a secretCertWriter. @@ -50,8 +44,6 @@ type SecretCertWriterOptions struct { CertGenerator generator.CertGenerator // secret points the secret that contains certificates that written by the CertWriter. Secret *types.NamespacedName - // Writer is used in dryrun mode for writing the objects in yaml format. - Writer io.Writer } var _ CertWriter = &secretCertWriter{} @@ -60,9 +52,6 @@ func (ops *SecretCertWriterOptions) setDefaults() { if ops.CertGenerator == nil { ops.CertGenerator = &generator.SelfSignedCertGenerator{} } - if ops.Writer == nil { - ops.Writer = os.Stdout - } } func (ops *SecretCertWriterOptions) validate() error { @@ -88,9 +77,8 @@ func NewSecretCertWriter(ops SecretCertWriterOptions) (CertWriter, error) { } // EnsureCert provisions certificates for a webhookClientConfig by writing the certificates to a k8s secret. -func (s *secretCertWriter) EnsureCert(dnsName string, dryrun bool) (*generator.Artifacts, bool, error) { +func (s *secretCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) { // Create or refresh the certs based on clientConfig - s.dryrun = dryrun s.dnsName = dnsName return handleCommon(s.dnsName, s) } @@ -111,9 +99,6 @@ func (s *secretCertWriter) write() (*generator.Artifacts, error) { if err != nil { return nil, err } - if s.dryrun { - return certs, s.dryrunWrite(secret) - } err = s.Client.Create(nil, secret) if apierrors.IsAlreadyExists(err) { return nil, alreadyExistError{err} @@ -127,32 +112,27 @@ func (s *secretCertWriter) overwrite() ( if err != nil { return nil, err } - if s.dryrun { - return certs, s.dryrunWrite(secret) - } err = s.Client.Update(nil, secret) return certs, err } -func (s *secretCertWriter) dryrunWrite(secret *corev1.Secret) error { - sec, err := yaml.Marshal(secret) - if err != nil { - return err - } - _, err = s.Writer.Write(sec) - return err -} - func (s *secretCertWriter) read() (*generator.Artifacts, error) { - if s.dryrun { - return nil, notFoundError{} + secret := &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, } - secret := &corev1.Secret{} err := s.Client.Get(nil, *s.Secret, secret) if apierrors.IsNotFound(err) { return nil, notFoundError{err} } - return secretToCerts(secret), err + certs := secretToCerts(secret) + if certs != nil { + // Store the CA for next usage. + s.CertGenerator.SetCA(certs.CAKey, certs.CACert) + } + return certs, nil } func secretToCerts(secret *corev1.Secret) *generator.Artifacts { @@ -160,6 +140,7 @@ func secretToCerts(secret *corev1.Secret) *generator.Artifacts { return nil } return &generator.Artifacts{ + CAKey: secret.Data[CAKeyName], CACert: secret.Data[CACertName], Cert: secret.Data[ServerCertName], Key: secret.Data[ServerKeyName], @@ -168,11 +149,16 @@ func secretToCerts(secret *corev1.Secret) *generator.Artifacts { func certsToSecret(certs *generator.Artifacts, sec types.NamespacedName) *corev1.Secret { return &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, ObjectMeta: metav1.ObjectMeta{ Namespace: sec.Namespace, Name: sec.Name, }, Data: map[string][]byte{ + CAKeyName: certs.CAKey, CACertName: certs.CACert, ServerKeyName: certs.Key, ServerCertName: certs.Cert, diff --git a/pkg/webhook/internal/cert/writer/secret_test.go b/pkg/webhook/internal/cert/writer/secret_test.go index 0090f9add1..bf6a224942 100644 --- a/pkg/webhook/internal/cert/writer/secret_test.go +++ b/pkg/webhook/internal/cert/writer/secret_test.go @@ -51,6 +51,7 @@ var _ = Describe("secretCertWriter", func() { CertGenerator: &fakegenerator.CertGenerator{ DNSNameToCertArtifacts: map[string]*generator.Artifacts{ dnsName: { + CAKey: []byte(`CAKeyBytes`), CACert: []byte(`CACertBytes`), Cert: []byte(`CertBytes`), Key: []byte(`KeyBytes`), @@ -66,7 +67,7 @@ var _ = Describe("secretCertWriter", func() { Context("Failed to EnsureCerts", func() { Describe("empty DNS name", func() { It("should return error", func() { - _, _, err := certWriter.EnsureCert("", false) + _, _, err := certWriter.EnsureCert("") Expect(err).To(MatchError("dnsName should not be empty")) }) }) @@ -78,6 +79,10 @@ var _ = Describe("secretCertWriter", func() { //isController := true //blockOwnerDeletion := true secret = &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, ObjectMeta: metav1.ObjectMeta{ Namespace: "namespace-bar", Name: "secret-foo", @@ -93,6 +98,7 @@ var _ = Describe("secretCertWriter", func() { //}, }, Data: map[string][]byte{ + CAKeyName: []byte(`CAKeyBytes`), CACertName: []byte(`CACertBytes`), ServerKeyName: []byte(`KeyBytes`), ServerCertName: []byte(`CertBytes`), @@ -103,7 +109,7 @@ var _ = Describe("secretCertWriter", func() { Context("certGenerator is not set", func() { It("should default it and return no error", func() { - _, _, err := certWriter.EnsureCert(dnsName, false) + _, _, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) list := &corev1.List{} err = sCertWriter.Client.List(nil, &client.ListOptions{ @@ -128,7 +134,7 @@ var _ = Describe("secretCertWriter", func() { }) It("should create new secrets with certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName, false) + _, changed, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) list := &corev1.List{} err = sCertWriter.Client.List(nil, &client.ListOptions{ @@ -166,7 +172,7 @@ var _ = Describe("secretCertWriter", func() { }) It("should replace with new certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName, false) + _, changed, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) list := &corev1.List{} err = sCertWriter.Client.List(nil, &client.ListOptions{ @@ -189,7 +195,8 @@ var _ = Describe("secretCertWriter", func() { BeforeEach(func(done Done) { oldSecret = secret.DeepCopy() oldSecret.Data = map[string][]byte{ - CACertName: []byte(`oldCACertBytes`), + CAKeyName: []byte(`invalidCAKeyBytes`), + CACertName: []byte(`invalidCACertBytes`), ServerKeyName: []byte(`oldKeyBytes`), ServerCertName: []byte(`oldCertBytes`), } @@ -198,7 +205,7 @@ var _ = Describe("secretCertWriter", func() { }) It("should replace with new certs", func() { - _, changed, err := certWriter.EnsureCert(dnsName, false) + _, changed, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) list := &corev1.List{} err = sCertWriter.Client.List(nil, &client.ListOptions{ @@ -221,6 +228,7 @@ var _ = Describe("secretCertWriter", func() { Context("cert is valid", func() { BeforeEach(func(done Done) { oldSecret.Data = map[string][]byte{ + CAKeyName: []byte(certs2.CAKey), CACertName: []byte(certs2.CACert), ServerKeyName: []byte(certs2.Key), ServerCertName: []byte(certs2.Cert), @@ -235,6 +243,7 @@ var _ = Describe("secretCertWriter", func() { BeforeEach(func(done Done) { oldSecret = secret.DeepCopy() oldSecret.Data = map[string][]byte{ + CAKeyName: []byte(certs2.CAKey), CACertName: []byte(certs2.CACert), ServerKeyName: []byte(certs2.Key), ServerCertName: []byte(certs2.Cert), @@ -246,7 +255,7 @@ var _ = Describe("secretCertWriter", func() { close(done) }) It("should keep the secret", func() { - _, changed, err := certWriter.EnsureCert(dnsName, false) + _, changed, err := certWriter.EnsureCert(dnsName) Expect(err).NotTo(HaveOccurred()) list := &corev1.List{} err = sCertWriter.Client.List(nil, &client.ListOptions{ @@ -270,6 +279,7 @@ var _ = Describe("secretCertWriter", func() { BeforeEach(func(done Done) { oldSecret = secret.DeepCopy() oldSecret.Data = map[string][]byte{ + CAKeyName: []byte(`oldCAKeyBytes`), CACertName: []byte(`oldCACertBytes`), //ServerKeyName: []byte(expiringKeyPEM), //ServerCertName: []byte(expiringCertPEM), diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 3c547c42eb..bae22685aa 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -54,10 +54,9 @@ type ServerOptions struct { // Client will be injected by the manager if not set. Client client.Client - // Dryrun controls if the server will install the webhookConfiguration and service if any. - // If true, it will print the objects in yaml format. - // If false, it will install the objects in the cluster. - Dryrun bool + // InstallWebhookConfig controls if the server will automatically create webhook related objects + // during bootstrapping. e.g. webhookConfiguration, service and secret. + InstallWebhookConfig bool // BootstrapOptions contains the options for bootstrapping the admission server. *BootstrapOptions @@ -75,7 +74,8 @@ type BootstrapOptions struct { // This is optional. If unspecified, it will write to the filesystem. // It the secret already exists and is different from the desired, it will be replaced. Secret *apitypes.NamespacedName - // Writer is used in dryrun mode for writing the objects in yaml format. + + // Deprecated: Writer will not be used anywhere. Writer io.Writer // Service is k8s service fronting the webhook server pod(s). @@ -187,13 +187,17 @@ func (s *Server) Handle(pattern string, handler http.Handler) { var _ manager.Runnable = &Server{} -// Start runs the server if s.Dryrun is false. -// Otherwise, it will print the objects in yaml format. +// Start runs the server. +// It will install the webhook related resources depend on the server configuration. func (s *Server) Start(stop <-chan struct{}) error { - err := s.installWebhookConfig() - // if encounter an error or it's in dryrun mode, return. - if err != nil || s.Dryrun { - return err + if s.InstallWebhookConfig { + log.Info("installing webhook configuration in cluster") + err := s.InstallWebhookManifests() + if err != nil { + return err + } + } else { + log.Info("webhook installer is disabled") } srv := &http.Server{ diff --git a/pkg/webhook/util.go b/pkg/webhook/util.go index 8ca0c270c7..47e2635ca5 100644 --- a/pkg/webhook/util.go +++ b/pkg/webhook/util.go @@ -18,7 +18,6 @@ package webhook import ( "context" - "fmt" admissionregistration "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" @@ -27,29 +26,34 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -type mutateFn func(current, desired runtime.Object) error +type mutateFn func(current, desired *runtime.Object) error -var serviceFn = func(current, desired runtime.Object) error { - typedC := current.(*corev1.Service) - typedD := desired.(*corev1.Service) +var serviceFn = func(current, desired *runtime.Object) error { + typedC := (*current).(*corev1.Service) + typedD := (*desired).(*corev1.Service) typedC.Spec.Selector = typedD.Spec.Selector return nil } -var mutatingWebhookConfigFn = func(current, desired runtime.Object) error { - typedC := current.(*admissionregistration.MutatingWebhookConfiguration) - typedD := desired.(*admissionregistration.MutatingWebhookConfiguration) +var mutatingWebhookConfigFn = func(current, desired *runtime.Object) error { + typedC := (*current).(*admissionregistration.MutatingWebhookConfiguration) + typedD := (*desired).(*admissionregistration.MutatingWebhookConfiguration) typedC.Webhooks = typedD.Webhooks return nil } -var validatingWebhookConfigFn = func(current, desired runtime.Object) error { - typedC := current.(*admissionregistration.ValidatingWebhookConfiguration) - typedD := desired.(*admissionregistration.ValidatingWebhookConfiguration) +var validatingWebhookConfigFn = func(current, desired *runtime.Object) error { + typedC := (*current).(*admissionregistration.ValidatingWebhookConfiguration) + typedD := (*desired).(*admissionregistration.ValidatingWebhookConfiguration) typedC.Webhooks = typedD.Webhooks return nil } +var genericFn = func(current, desired *runtime.Object) error { + *current = *desired + return nil +} + // createOrReplaceHelper creates the object if it doesn't exist; // otherwise, it will replace it. // When replacing, fn should know how to preserve existing fields in the object GET from the APIServer. @@ -70,7 +74,7 @@ func createOrReplaceHelper(c client.Client, obj runtime.Object, fn mutateFn) err if err != nil { return err } - err = fn(existing, obj) + err = fn(&existing, &obj) if err != nil { return err } @@ -83,6 +87,7 @@ func createOrReplaceHelper(c client.Client, obj runtime.Object, fn mutateFn) err // otherwise, it will replace it. // When replacing, it knows how to preserve existing fields in the object GET from the APIServer. // It currently only support MutatingWebhookConfiguration, ValidatingWebhookConfiguration and Service. +// For other kinds, it uses genericFn to replace the whole object. func createOrReplace(c client.Client, obj runtime.Object) error { if obj == nil { return nil @@ -95,7 +100,7 @@ func createOrReplace(c client.Client, obj runtime.Object) error { case *corev1.Service: return createOrReplaceHelper(c, obj, serviceFn) default: - return fmt.Errorf("unsupported GroupVersionKind: %#v", obj.GetObjectKind().GroupVersionKind()) + return createOrReplaceHelper(c, obj, genericFn) } }