Skip to content

Commit d561f55

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#260 from mengqiy/fixwebhookserver
🐛 fix issue when webhook server refreshing cert
2 parents 752e0b5 + 09998a3 commit d561f55

File tree

4 files changed

+249
-12
lines changed

4 files changed

+249
-12
lines changed

‎Gopkg.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/webhook/server.go

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727

2828
"k8s.io/apimachinery/pkg/runtime"
2929
apitypes "k8s.io/apimachinery/pkg/types"
30+
"k8s.io/apimachinery/pkg/util/wait"
3031
"sigs.k8s.io/controller-runtime/pkg/client"
3132
"sigs.k8s.io/controller-runtime/pkg/manager"
3233
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
@@ -36,6 +37,9 @@ import (
3637
"sigs.k8s.io/controller-runtime/pkg/webhook/types"
3738
)
3839

40+
// default interval for checking cert is 90 days (~3 months)
41+
var defaultCertRefreshInterval = 3 * 30 * 24 * time.Hour
42+
3943
// ServerOptions are options for configuring an admission webhook server.
4044
type ServerOptions struct {
4145
// Port is the port number that the server will serve.
@@ -128,6 +132,9 @@ type Server struct {
128132
// manager is the manager that this webhook server will be registered.
129133
manager manager.Manager
130134

135+
// httpServer is the actual server that serves the traffic.
136+
httpServer *http.Server
137+
131138
once sync.Once
132139
}
133140

@@ -209,21 +216,21 @@ func (s *Server) Start(stop <-chan struct{}) error {
209216
return s.run(stop)
210217
}
211218

212-
func (s *Server) run(stop <-chan struct{}) error {
213-
srv := &http.Server{
214-
Addr: fmt.Sprintf(":%v", s.Port),
215-
Handler: s.sMux,
216-
}
219+
func (s *Server) run(stop <-chan struct{}) error { // nolint: gocyclo
217220
errCh := make(chan error)
218221
serveFn := func() {
219-
errCh <- srv.ListenAndServeTLS(path.Join(s.CertDir, writer.ServerCertName), path.Join(s.CertDir, writer.ServerKeyName))
222+
s.httpServer = &http.Server{
223+
Addr: fmt.Sprintf(":%v", s.Port),
224+
Handler: s.sMux,
225+
}
226+
log.Info("starting the webhook server.")
227+
errCh <- s.httpServer.ListenAndServeTLS(path.Join(s.CertDir, writer.ServerCertName), path.Join(s.CertDir, writer.ServerKeyName))
220228
}
221229

230+
shutdownHappend := false
231+
timer := time.Tick(wait.Jitter(defaultCertRefreshInterval, 0.1))
222232
go serveFn()
223233
for {
224-
// TODO(mengqiy): add jitter to the timer
225-
// Could use https://godoc.org/k8s.io/apimachinery/pkg/util/wait#Jitter
226-
timer := time.Tick(6 * 30 * 24 * time.Hour)
227234
select {
228235
case <-timer:
229236
changed, err := s.RefreshCert()
@@ -232,19 +239,28 @@ func (s *Server) run(stop <-chan struct{}) error {
232239
return err
233240
}
234241
if !changed {
242+
log.Info("no need to reload the certificates.")
235243
continue
236244
}
237245
log.Info("server is shutting down to reload the certificates.")
238-
err = srv.Shutdown(context.Background())
246+
shutdownHappend = true
247+
err = s.httpServer.Shutdown(context.Background())
239248
if err != nil {
240249
log.Error(err, "encountering error when shutting down")
241250
return err
242251
}
252+
timer = time.Tick(wait.Jitter(defaultCertRefreshInterval, 0.1))
243253
go serveFn()
244254
case <-stop:
245-
return nil
255+
return s.httpServer.Shutdown(context.Background())
246256
case e := <-errCh:
247-
return e
257+
// Don't exit when getting an http.ErrServerClosed error due to restarting the server.
258+
if shutdownHappend && e == http.ErrServerClosed {
259+
shutdownHappend = false
260+
} else if e != nil {
261+
log.Error(e, "server returns an unexpected error")
262+
return e
263+
}
248264
}
249265
}
250266
}

‎pkg/webhook/server_test.go

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package webhook
18+
19+
import (
20+
"context"
21+
"io/ioutil"
22+
"net/http"
23+
"os"
24+
"time"
25+
26+
. "github.com/onsi/ginkgo"
27+
. "github.com/onsi/gomega"
28+
29+
"k8s.io/apimachinery/pkg/runtime"
30+
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert"
31+
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/generator"
32+
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/cert/writer"
33+
"sigs.k8s.io/testing_frameworks/integration/addr"
34+
)
35+
36+
type fakeCertWriter struct {
37+
changed bool
38+
}
39+
40+
func (cw *fakeCertWriter) EnsureCert(dnsName string) (*generator.Artifacts, bool, error) {
41+
return &generator.Artifacts{}, cw.changed, nil
42+
}
43+
44+
func (cw *fakeCertWriter) Inject(objs ...runtime.Object) error {
45+
return nil
46+
}
47+
48+
var _ = Describe("webhook server", func() {
49+
Describe("run", func() {
50+
var stop chan struct{}
51+
var s *Server
52+
var cn = "example.com"
53+
54+
BeforeEach(func() {
55+
port, _, err := addr.Suggest()
56+
Expect(err).NotTo(HaveOccurred())
57+
s = &Server{
58+
sMux: http.NewServeMux(),
59+
ServerOptions: ServerOptions{
60+
Port: int32(port),
61+
BootstrapOptions: &BootstrapOptions{
62+
Host: &cn,
63+
},
64+
},
65+
}
66+
67+
cg := &generator.SelfSignedCertGenerator{}
68+
s.CertDir, err = ioutil.TempDir("/tmp", "controller-runtime-")
69+
Expect(err).NotTo(HaveOccurred())
70+
certWriter, err := writer.NewFSCertWriter(writer.FSCertWriterOptions{CertGenerator: cg, Path: s.CertDir})
71+
Expect(err).NotTo(HaveOccurred())
72+
_, _, err = certWriter.EnsureCert(cn)
73+
Expect(err).NotTo(HaveOccurred())
74+
75+
stop = make(chan struct{})
76+
})
77+
78+
It("should stop if the stop channel is closed", func() {
79+
var e error
80+
go func() {
81+
defer GinkgoRecover()
82+
e = s.run(stop)
83+
}()
84+
85+
Eventually(func() *http.Server {
86+
return s.httpServer
87+
}).ShouldNot(BeNil())
88+
89+
close(stop)
90+
Expect(e).NotTo(HaveOccurred())
91+
})
92+
93+
It("should exit if the server encounter an unexpected error", func() {
94+
var e error
95+
go func() {
96+
defer GinkgoRecover()
97+
e = s.run(stop)
98+
}()
99+
100+
Eventually(func() *http.Server {
101+
return s.httpServer
102+
}).ShouldNot(BeNil())
103+
104+
err := s.httpServer.Shutdown(context.Background())
105+
Expect(err).NotTo(HaveOccurred())
106+
107+
Eventually(func() error {
108+
return e
109+
}).Should(Equal(http.ErrServerClosed))
110+
111+
close(stop)
112+
})
113+
114+
It("should be able to keep existing valid cert when timer fires", func() {
115+
var e error
116+
defaultCertRefreshInterval = 500 * time.Millisecond
117+
118+
s.certProvisioner = &cert.Provisioner{
119+
CertWriter: &fakeCertWriter{changed: false},
120+
}
121+
122+
go func() {
123+
defer GinkgoRecover()
124+
e = s.run(stop)
125+
}()
126+
127+
// Wait for multiple cycles of timer firing
128+
time.Sleep(2 * time.Second)
129+
Expect(e).NotTo(HaveOccurred())
130+
131+
close(stop)
132+
})
133+
134+
It("should be able to rotate the cert when timer fires", func() {
135+
var e error
136+
defaultCertRefreshInterval = 500 * time.Millisecond
137+
s.certProvisioner = &cert.Provisioner{
138+
CertWriter: &fakeCertWriter{changed: true},
139+
}
140+
141+
go func() {
142+
defer GinkgoRecover()
143+
e = s.run(stop)
144+
}()
145+
146+
Eventually(func() *http.Server {
147+
return s.httpServer
148+
}).ShouldNot(BeNil())
149+
150+
// Wait for multiple cycles of timer firing
151+
time.Sleep(2 * time.Second)
152+
Expect(e).NotTo(HaveOccurred())
153+
154+
close(stop)
155+
})
156+
157+
AfterEach(func() {
158+
defaultCertRefreshInterval = 3 * 30 * 24 * time.Hour
159+
err := os.RemoveAll(s.CertDir)
160+
Expect(err).NotTo(HaveOccurred())
161+
}, 60)
162+
})
163+
})

‎pkg/webhook/webhook_suite_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package webhook
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/ginkgo"
23+
. "github.com/onsi/gomega"
24+
25+
"k8s.io/client-go/kubernetes"
26+
"k8s.io/client-go/rest"
27+
"sigs.k8s.io/controller-runtime/pkg/envtest"
28+
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
29+
)
30+
31+
func TestSource(t *testing.T) {
32+
RegisterFailHandler(Fail)
33+
RunSpecsWithDefaultAndCustomReporters(t, "Webhook Integration Suite", []Reporter{envtest.NewlineReporter{}})
34+
}
35+
36+
var testenv *envtest.Environment
37+
var cfg *rest.Config
38+
var clientset *kubernetes.Clientset
39+
40+
var _ = BeforeSuite(func(done Done) {
41+
logf.SetLogger(logf.ZapLoggerTo(GinkgoWriter, true))
42+
43+
testenv = &envtest.Environment{}
44+
45+
var err error
46+
cfg, err = testenv.Start()
47+
Expect(err).NotTo(HaveOccurred())
48+
49+
clientset, err = kubernetes.NewForConfig(cfg)
50+
Expect(err).NotTo(HaveOccurred())
51+
52+
close(done)
53+
}, 60)
54+
55+
var _ = AfterSuite(func() {
56+
testenv.Stop()
57+
})

0 commit comments

Comments
 (0)