Merge pull request #145 from mengqiy/fix_fs_cert_writer

fix issues when running on master

pkg/webhook/bootstrap.go

@@ -19,10 +19,12 @@ package webhook
 import (
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"net/url"
 	"os"
 	"path"
+	"strconv"
 
 	"github.com/ghodss/yaml"
 
@@ -187,7 +189,7 @@ func (s *Server) getClientConfig() (*admissionregistration.WebhookClientConfig,
 	if s.Host != nil {
 		u := url.URL{
 			Scheme: "https",
-			Host:   *s.Host,
+			Host:   net.JoinHostPort(*s.Host, strconv.Itoa(int(s.Port))),
 		}
 		urlString := u.String()
 		cc.URL = &urlString
@@ -332,7 +334,7 @@ func (s *Server) admissionWebhook(path string, wh *admission.Webhook) (*admissio
 }
 
 // service creates a corev1.service object fronting the admission server.
-func (s *Server) service() *corev1.Service {
+func (s *Server) service() runtime.Object {
 	if s.Service == nil {
 		return nil
 	}

pkg/webhook/internal/cert/provisioner.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"net"
 	"net/url"
 
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
@@ -127,5 +128,12 @@ func dnsNameFromClientConfig(config *admissionregistrationv1beta1.WebhookClientC
 		return generator.ServiceToCommonName(config.Service.Namespace, config.Service.Name), nil
 	}
 	u, err := url.Parse(*config.URL)
-	return u.Host, err
+	if err != nil {
+		return "", err
+	}
+	host, _, err := net.SplitHostPort(u.Host)
+	if err != nil {
+		return u.Host, nil
+	}
+	return host, err
 }

pkg/webhook/internal/cert/provisioner_test.go

@@ -191,6 +191,16 @@ var _ = Describe("dnsNameFromClientConfig", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(dnsName).To(Equal("foo.example.com"))
 		})
+
+		It("should return a DNS name w/o port", func() {
+			urlStr := "https://foo.example.com:9876/webhookendpoint"
+			cc := &admissionregistrationv1beta1.WebhookClientConfig{
+				URL: &urlStr,
+			}
+			dnsName, err := dnsNameFromClientConfig(cc)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(dnsName).To(Equal("foo.example.com"))
+		})
 	})
 })
 

pkg/webhook/internal/cert/writer/fs.go

@@ -18,6 +18,7 @@ package writer
 
 import (
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path"
@@ -91,21 +92,39 @@ func (f *fsCertWriter) doWrite() (*generator.Artifacts, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// AtomicWriter's algorithm only manages files using symbolic link.
+	// If a file is not a symbolic link, will ignore the update for it.
+	// We want to cleanup for AtomicWriter by removing old files that are not symbolic links.
+	err = prepareToWrite(f.Path)
+	if err != nil {
+		return nil, err
+	}
+
 	aw, err := atomic.NewAtomicWriter(f.Path, log.WithName("atomic-writer").
 		WithValues("task", "processing webhook"))
 	if err != nil {
 		return nil, err
 	}
-	// AtomicWriter's algorithm only manages files using symbolic link.
-	// If a file is not a symbolic link, will ignore the update for it.
-	// We want to cleanup for AtomicWriter by removing old files that are not symbolic links.
-	prepareToWrite(f.Path)
 	err = aw.Write(certToProjectionMap(certs))
 	return certs, err
 }
 
 // prepareToWrite ensures it directory is compatible with the atomic.Writer library.
-func prepareToWrite(dir string) {
+func prepareToWrite(dir string) error {
+	_, err := os.Stat(dir)
+	switch {
+	case os.IsNotExist(err):
+		log.Info(fmt.Sprintf("cert directory %v doesn't exist, creating", dir))
+		// TODO: figure out if we can reduce the permission. (Now it's 0777)
+		err = os.MkdirAll(dir, 0777)
+		if err != nil {
+			return fmt.Errorf("can't create dir: %v", dir)
+		}
+	case err != nil:
+		return err
+	}
+
 	filenames := []string{CACertName, ServerCertName, ServerKeyName}
 	for _, f := range filenames {
 		abspath := path.Join(dir, f)
@@ -124,6 +143,7 @@ func prepareToWrite(dir string) {
 			}
 		}
 	}
+	return nil
 }
 
 func (f *fsCertWriter) read() (*generator.Artifacts, error) {

pkg/webhook/util.go

@@ -84,6 +84,9 @@ func createOrReplaceHelper(c client.Client, obj runtime.Object, fn mutateFn) err
 // When replacing, it knows how to preserve existing fields in the object GET from the APIServer.
 // It currently only support MutatingWebhookConfiguration, ValidatingWebhookConfiguration and Service.
 func createOrReplace(c client.Client, obj runtime.Object) error {
+	if obj == nil {
+		return nil
+	}
 	switch obj.(type) {
 	case *admissionregistration.MutatingWebhookConfiguration:
 		return createOrReplaceHelper(c, obj, mutatingWebhookConfigFn)