diff --git a/go.sum b/go.sum index 790d748e..fb22470c 100644 --- a/go.sum +++ b/go.sum @@ -100,6 +100,7 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/daixiang0/gci v0.0.0-20200727065011-66f1df783cb2 h1:3Lhhps85OdA8ezsEKu+IA1hE+DBTjt/fjd7xNCrHbVA= github.com/daixiang0/gci v0.0.0-20200727065011-66f1df783cb2/go.mod h1:+AV8KmHTGxxwp/pY84TLQfFKp2vuKXXJVzF3kD/hfR4= +github.com/daixiang0/gci v0.2.4 h1:BUCKk5nlK2m+kRIsoj+wb/5hazHvHeZieBKWd9Afa8Q= github.com/daixiang0/gci v0.2.4/go.mod h1:+AV8KmHTGxxwp/pY84TLQfFKp2vuKXXJVzF3kD/hfR4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -134,6 +135,7 @@ github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2H github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/go-critic/go-critic v0.5.0 h1:Ic2p5UCl5fX/2WX2w8nroPpPhxRNsNTMlJzsu/uqwnM= github.com/go-critic/go-critic v0.5.0/go.mod h1:4jeRh3ZAVnRYhuWdOEvwzVqLUpxMSoAT0xZ74JsTPlo= +github.com/go-critic/go-critic v0.5.2 h1:3RJdgf6u4NZUumoP8nzbqiiNT8e1tC2Oc7jlgqre/IA= github.com/go-critic/go-critic v0.5.2/go.mod h1:cc0+HvdE3lFpqLecgqMaJcvWWH77sLdBp+wLGPM1Yyo= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= @@ -197,6 +199,7 @@ github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y= github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8= github.com/gofrs/flock v0.7.1 h1:DP+LD/t0njgoPBvT5MJLeliUIVQR03hiKR6vezdwHlc= github.com/gofrs/flock v0.7.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= +github.com/gofrs/flock v0.8.0 h1:MSdYClljsF3PbENUUEx85nkWfJSGfzYI9yEBZOJz6CY= github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= @@ -435,6 +438,7 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nishanths/exhaustive v0.0.0-20200708172631-8866003e3856 h1:W3KBC2LFyfgd+wNudlfgCCsTo4q97MeNWrfz8/wSdSc= github.com/nishanths/exhaustive v0.0.0-20200708172631-8866003e3856/go.mod h1:wBEpHwM2OdmeNpdCvRPUlkEbBuaFmcK4Wv8Q7FuGW3c= +github.com/nishanths/exhaustive v0.0.0-20200811152831-6cf413ae40e0 h1:eMV1t2NQRc3r1k3guWiv/zEeqZZP6kPvpUfy6byfL1g= github.com/nishanths/exhaustive v0.0.0-20200811152831-6cf413ae40e0/go.mod h1:wBEpHwM2OdmeNpdCvRPUlkEbBuaFmcK4Wv8Q7FuGW3c= github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= @@ -490,6 +494,7 @@ github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40T github.com/quasilyte/go-consistent v0.0.0-20190521200055-c6f3937de18c/go.mod h1:5STLWrekHfjyYwxBRVRXNOSewLJ3PWfDJd1VyTS21fI= github.com/quasilyte/go-ruleguard v0.1.2-0.20200318202121-b00d7a75d3d8 h1:DvnesvLtRPQOvaUbfXfh0tpMHg29by0H7F2U+QIkSu8= github.com/quasilyte/go-ruleguard v0.1.2-0.20200318202121-b00d7a75d3d8/go.mod h1:CGFX09Ci3pq9QZdj86B+VGIdNj4VyCo2iPOGS9esB/k= +github.com/quasilyte/go-ruleguard v0.2.0 h1:UOVMyH2EKkxIfzrULvA9n/tO+HtEhqD9mrLSWMr5FwU= github.com/quasilyte/go-ruleguard v0.2.0/go.mod h1:2RT/tf0Ce0UDj5y243iWKosQogJd8+1G3Rs2fxmlYnw= github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95 h1:L8QM9bvf68pVdQ3bCFZMDmnt9yqcMBro1pC7F+IPYMY= github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95/go.mod h1:rlzQ04UMyJXu/aOvhd8qT+hvDrFpiwqp8MRXDY9szc0= @@ -531,6 +536,7 @@ github.com/sonatard/noctx v0.0.1 h1:VC1Qhl6Oxx9vvWo3UDgrGXYCeKCe3Wbw7qAWL6FrmTY= github.com/sonatard/noctx v0.0.1/go.mod h1:9D2D/EoULe8Yy2joDHJj7bv3sZoq9AaSb8B4lqBjiZI= github.com/sourcegraph/go-diff v0.5.3 h1:lhIKJ2nXLZZ+AfbHpYxTn0pXpNTTui0DX7DO3xeb1Zs= github.com/sourcegraph/go-diff v0.5.3/go.mod h1:v9JDtjCE4HHHCZGId75rg8gkKKa98RVjBcBGsVmMmak= +github.com/sourcegraph/go-diff v0.6.0 h1:WbN9e/jD8ujU+o0vd9IFN5AEwtfB0rn/zM/AANaClqQ= github.com/sourcegraph/go-diff v0.6.0/go.mod h1:iBszgVvyxdc8SFZ7gm69go2KDdt3ag071iBaWPF6cjs= 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= @@ -551,9 +557,11 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/spf13/viper v1.7.0 h1:xVKxvI7ouOI5I+U9s2eeiUfMaWBVoXA3AWskkrqK0VM= github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= +github.com/spf13/viper v1.7.1 h1:pM5oEahlgWv/WnHXpgbKz7iLIxRf65tye2Ci+XFK5sk= github.com/spf13/viper v1.7.1/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= github.com/ssgreg/nlreturn/v2 v2.0.1 h1:+lm6xFjVuNw/9t/Fh5sIwfNWefiD5bddzc6vwJ1TvRI= github.com/ssgreg/nlreturn/v2 v2.0.1/go.mod h1:E/iiPB78hV7Szg2YfRgyIrk1AD6JVMTRkkxBiELzh2I= +github.com/ssgreg/nlreturn/v2 v2.1.0 h1:6/s4Rc49L6Uo6RLjhWZGBpWWjfzk2yrf1nIW8m4wgVA= github.com/ssgreg/nlreturn/v2 v2.1.0/go.mod h1:E/iiPB78hV7Szg2YfRgyIrk1AD6JVMTRkkxBiELzh2I= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -581,6 +589,7 @@ github.com/tommy-muehle/go-mnd v1.3.1-0.20200224220436-e6f9a994e8fa/go.mod h1:dS github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ultraware/funlen v0.0.2 h1:Av96YVBwwNSe4MLR7iI/BIa3VyI7/djnto/pK3Uxbdo= github.com/ultraware/funlen v0.0.2/go.mod h1:Dp4UiAus7Wdb9KUZsYWZEWiRzGuM2kXM1lPbfaF6xhA= +github.com/ultraware/funlen v0.0.3 h1:5ylVWm8wsNwH5aWo9438pwvsK0QiqVuUrt9bn7S/iLA= github.com/ultraware/funlen v0.0.3/go.mod h1:Dp4UiAus7Wdb9KUZsYWZEWiRzGuM2kXM1lPbfaF6xhA= github.com/ultraware/whitespace v0.0.4 h1:If7Va4cM03mpgrNH9k49/VOicWpGoG70XPBFFODYDsg= github.com/ultraware/whitespace v0.0.4/go.mod h1:aVMh/gQve5Maj9hQ/hg+F75lr/X5A89uZnzAmWSineA= @@ -784,11 +793,13 @@ golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20200701041122-1837592efa10/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200724022722-7017fd6b1305 h1:yaM5S0KcY0lIoZo7Fl+oi91b/DdlU2zuWpfHrpWbCS0= golang.org/x/tools v0.0.0-20200724022722-7017fd6b1305/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= +golang.org/x/tools v0.0.0-20200812195022-5ae4c3c160a0 h1:SQvH+DjrwqD1hyyQU+K7JegHz1KEZgEwt17p9d6R2eg= golang.org/x/tools v0.0.0-20200812195022-5ae4c3c160a0/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M= @@ -872,6 +883,7 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= +honnef.co/go/tools v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k= honnef.co/go/tools v0.0.1-2020.1.5/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= k8s.io/api v0.19.0 h1:XyrFIJqTYZJ2DU7FBE/bSPz7b1HvbVBuBf07oeo6eTc= k8s.io/api v0.19.0/go.mod h1:I1K45XlvTrDjmj5LoM5LuP/KYrhWbjUKT/SoPG0qTjw= diff --git a/internal/controller/apicerts/apiservice_updater.go b/internal/controller/apicerts/apiservice_updater.go new file mode 100644 index 00000000..af31f238 --- /dev/null +++ b/internal/controller/apicerts/apiservice_updater.go @@ -0,0 +1,69 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package apicerts + +import ( + "fmt" + + k8serrors "k8s.io/apimachinery/pkg/api/errors" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/klog/v2" + aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" + + pinnipedcontroller "github.com/suzerain-io/pinniped/internal/controller" + "github.com/suzerain-io/pinniped/internal/controllerlib" +) + +type apiServiceUpdaterController struct { + namespace string + aggregatorClient aggregatorclient.Interface + secretInformer corev1informers.SecretInformer +} + +func NewAPIServiceUpdaterController( + namespace string, + aggregatorClient aggregatorclient.Interface, + secretInformer corev1informers.SecretInformer, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + return controllerlib.New( + controllerlib.Config{ + Name: "certs-manager-controller", + Syncer: &apiServiceUpdaterController{ + namespace: namespace, + aggregatorClient: aggregatorClient, + secretInformer: secretInformer, + }, + }, + withInformer( + secretInformer, + pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(certsSecretName, namespace), + controllerlib.InformerOption{}, + ), + ) +} + +func (c *apiServiceUpdaterController) Sync(ctx controllerlib.Context) error { + // Try to get the secret from the informer cache. + certSecret, err := c.secretInformer.Lister().Secrets(c.namespace).Get(certsSecretName) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, certsSecretName, err) + } + if notFound { + // The secret does not exist yet, so nothing to do. + klog.Info("apiServiceUpdaterController Sync found that the secret does not exist yet or was deleted") + return nil + } + + // Update the APIService to give it the new CA bundle. + if err := UpdateAPIService(ctx.Context, c.aggregatorClient, certSecret.Data[caCertificateSecretKey]); err != nil { + return fmt.Errorf("could not update the API service: %w", err) + } + + klog.Info("apiServiceUpdaterController Sync successfully updated API service") + return nil +} diff --git a/internal/controller/apicerts/apiservice_updater_test.go b/internal/controller/apicerts/apiservice_updater_test.go new file mode 100644 index 00000000..1a1bf55e --- /dev/null +++ b/internal/controller/apicerts/apiservice_updater_test.go @@ -0,0 +1,277 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package apicerts + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + kubeinformers "k8s.io/client-go/informers" + kubernetesfake "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + aggregatorfake "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake" + + pinnipedv1alpha1 "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped/v1alpha1" + "github.com/suzerain-io/pinniped/internal/controllerlib" + "github.com/suzerain-io/pinniped/internal/testutil" +) + +func TestAPIServiceUpdaterControllerOptions(t *testing.T) { + spec.Run(t, "options", func(t *testing.T, when spec.G, it spec.S) { + const installedInNamespace = "some-namespace" + + var r *require.Assertions + var observableWithInformerOption *testutil.ObservableWithInformerOption + var secretsInformerFilter controllerlib.Filter + + it.Before(func() { + r = require.New(t) + observableWithInformerOption = testutil.NewObservableWithInformerOption() + secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets() + _ = NewAPIServiceUpdaterController( + installedInNamespace, + nil, + secretsInformer, + observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters + ) + secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer) + }) + + when("watching Secret objects", func() { + var subject controllerlib.Filter + var target, wrongNamespace, wrongName, unrelated *corev1.Secret + + it.Before(func() { + subject = secretsInformerFilter + target = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "api-serving-cert", Namespace: installedInNamespace}} + wrongNamespace = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "api-serving-cert", Namespace: "wrong-namespace"}} + wrongName = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: installedInNamespace}} + unrelated = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}} + }) + + when("the target Secret changes", func() { + it("returns true to trigger the sync method", func() { + r.True(subject.Add(target)) + r.True(subject.Update(target, unrelated)) + r.True(subject.Update(unrelated, target)) + r.True(subject.Delete(target)) + }) + }) + + when("a Secret from another namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongNamespace)) + r.False(subject.Update(wrongNamespace, unrelated)) + r.False(subject.Update(unrelated, wrongNamespace)) + r.False(subject.Delete(wrongNamespace)) + }) + }) + + when("a Secret with a different name changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(wrongName)) + r.False(subject.Update(wrongName, unrelated)) + r.False(subject.Update(unrelated, wrongName)) + r.False(subject.Delete(wrongName)) + }) + }) + + when("a Secret with a different name and a different namespace changes", func() { + it("returns false to avoid triggering the sync method", func() { + r.False(subject.Add(unrelated)) + r.False(subject.Update(unrelated, unrelated)) + r.False(subject.Delete(unrelated)) + }) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) +} + +func TestAPIServiceUpdaterControllerSync(t *testing.T) { + spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { + const installedInNamespace = "some-namespace" + + var r *require.Assertions + + var subject controllerlib.Controller + var aggregatorAPIClient *aggregatorfake.Clientset + var kubeInformerClient *kubernetesfake.Clientset + var kubeInformers kubeinformers.SharedInformerFactory + var timeoutContext context.Context + var timeoutContextCancel context.CancelFunc + var syncContext *controllerlib.Context + + // Defer starting the informers until the last possible moment so that the + // nested Before's can keep adding things to the informer caches. + var startInformersAndController = func() { + // Set this at the last second to allow for injection of server override. + subject = NewAPIServiceUpdaterController( + installedInNamespace, + aggregatorAPIClient, + kubeInformers.Core().V1().Secrets(), + controllerlib.WithInformer, + ) + + // Set this at the last second to support calling subject.Name(). + syncContext = &controllerlib.Context{ + Context: timeoutContext, + Name: subject.Name(), + Key: controllerlib.Key{ + Namespace: installedInNamespace, + Name: "api-serving-cert", + }, + } + + // Must start informers before calling TestRunSynchronously() + kubeInformers.Start(timeoutContext.Done()) + controllerlib.TestRunSynchronously(t, subject) + } + + it.Before(func() { + r = require.New(t) + + timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3) + + kubeInformerClient = kubernetesfake.NewSimpleClientset() + kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0) + aggregatorAPIClient = aggregatorfake.NewSimpleClientset() + }) + + it.After(func() { + timeoutContextCancel() + }) + + when("there is not yet an api-serving-cert Secret in the installation namespace or it was deleted", func() { + it.Before(func() { + unrelatedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some other secret", + Namespace: installedInNamespace, + }, + } + err := kubeInformerClient.Tracker().Add(unrelatedSecret) + r.NoError(err) + }) + + it("does not need to make any API calls with its API client", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + r.Empty(aggregatorAPIClient.Actions()) + }) + }) + + when("there is an api-serving-cert Secret already in the installation namespace", func() { + it.Before(func() { + apiServingCertSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "api-serving-cert", + Namespace: installedInNamespace, + }, + Data: map[string][]byte{ + "caCertificate": []byte("fake CA cert"), + "tlsPrivateKey": []byte("fake private key"), + "tlsCertificateChain": []byte("fake cert chain"), + }, + } + err := kubeInformerClient.Tracker().Add(apiServingCertSecret) + r.NoError(err) + }) + + when("the APIService exists", func() { + it.Before(func() { + apiService := &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{ + Name: pinnipedv1alpha1.SchemeGroupVersion.Version + "." + pinnipedv1alpha1.GroupName, + }, + Spec: apiregistrationv1.APIServiceSpec{ + CABundle: nil, + VersionPriority: 1234, + }, + } + err := aggregatorAPIClient.Tracker().Add(apiService) + r.NoError(err) + }) + + it("updates the APIService's ca bundle", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) + + // Make sure we updated the APIService caBundle and left it otherwise unchanged + r.Len(aggregatorAPIClient.Actions(), 2) + r.Equal("get", aggregatorAPIClient.Actions()[0].GetVerb()) + expectedAPIServiceName := pinnipedv1alpha1.SchemeGroupVersion.Version + "." + pinnipedv1alpha1.GroupName + expectedUpdateAction := coretesting.NewUpdateAction( + schema.GroupVersionResource{ + Group: apiregistrationv1.GroupName, + Version: "v1", + Resource: "apiservices", + }, + "", + &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{ + Name: expectedAPIServiceName, + Namespace: "", + }, + Spec: apiregistrationv1.APIServiceSpec{ + VersionPriority: 1234, // only the CABundle is updated, this other field is left unchanged + CABundle: []byte("fake CA cert"), + }, + }, + ) + r.Equal(expectedUpdateAction, aggregatorAPIClient.Actions()[1]) + }) + + when("updating the APIService fails", func() { + it.Before(func() { + aggregatorAPIClient.PrependReactor( + "update", + "apiservices", + func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("update failed") + }, + ) + }) + + it("returns the update error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.EqualError(err, "could not update the API service: could not update API service: update failed") + }) + }) + }) + + when("the APIService does not exist", func() { + it.Before(func() { + unrelatedAPIService := &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: "some other api service"}, + Spec: apiregistrationv1.APIServiceSpec{}, + } + err := aggregatorAPIClient.Tracker().Add(unrelatedAPIService) + r.NoError(err) + }) + + it("returns an error", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.Error(err) + r.Regexp("could not get existing version of API service: .* not found", err.Error()) + }) + }) + }) + }, spec.Parallel(), spec.Report(report.Terminal{})) +} diff --git a/internal/controller/apicerts/certs_expirer.go b/internal/controller/apicerts/certs_expirer.go index 52e85aba..9cfd3ab3 100644 --- a/internal/controller/apicerts/certs_expirer.go +++ b/internal/controller/apicerts/certs_expirer.go @@ -69,7 +69,7 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error { return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, certsSecretName, err) } if notFound { - klog.Info("certsExpirerController Sync() found that the secret does not exist yet or was deleted") + klog.Info("certsExpirerController Sync found that the secret does not exist yet or was deleted") return nil } @@ -78,13 +78,13 @@ func (c *certsExpirerController) Sync(ctx controllerlib.Context) error { // If we can't read the cert, then really all we can do is log something, // since if we returned an error then the controller lib would just call us // again and again, which would probably yield the same results. - klog.Warningf("certsExpirerController Sync() found that the secret is malformed: %s", err.Error()) + klog.Warningf("certsExpirerController Sync found that the secret is malformed: %s", err.Error()) return nil } certAge := time.Since(notBefore) renewDelta := certAge - c.renewBefore - klog.Infof("certsExpirerController Sync() found a renew delta of %s", renewDelta) + klog.Infof("certsExpirerController Sync found a renew delta of %s", renewDelta) if renewDelta >= 0 || time.Now().After(notAfter) { err := c.k8sClient. CoreV1(). diff --git a/internal/controller/apicerts/certs_manager.go b/internal/controller/apicerts/certs_manager.go index db0503ce..4432ba45 100644 --- a/internal/controller/apicerts/certs_manager.go +++ b/internal/controller/apicerts/certs_manager.go @@ -16,7 +16,6 @@ import ( corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" - aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" "github.com/suzerain-io/pinniped/internal/certauthority" pinnipedcontroller "github.com/suzerain-io/pinniped/internal/controller" @@ -32,34 +31,37 @@ const ( ) type certsManagerController struct { - namespace string - k8sClient kubernetes.Interface - aggregatorClient aggregatorclient.Interface - secretInformer corev1informers.SecretInformer + namespace string + k8sClient kubernetes.Interface + secretInformer corev1informers.SecretInformer // certDuration is the lifetime of both the serving certificate and its CA // certificate that this controller will use when issuing the certificates. certDuration time.Duration + + generatedCACommonName string + serviceNameForGeneratedCertCommonName string } -func NewCertsManagerController( - namespace string, +func NewCertsManagerController(namespace string, k8sClient kubernetes.Interface, - aggregatorClient aggregatorclient.Interface, secretInformer corev1informers.SecretInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, withInitialEvent pinnipedcontroller.WithInitialEventOptionFunc, certDuration time.Duration, + generatedCACommonName string, + serviceNameForGeneratedCertCommonName string, ) controllerlib.Controller { return controllerlib.New( controllerlib.Config{ Name: "certs-manager-controller", Syncer: &certsManagerController{ - namespace: namespace, - k8sClient: k8sClient, - aggregatorClient: aggregatorClient, - secretInformer: secretInformer, - certDuration: certDuration, + namespace: namespace, + k8sClient: k8sClient, + secretInformer: secretInformer, + certDuration: certDuration, + generatedCACommonName: generatedCACommonName, + serviceNameForGeneratedCertCommonName: serviceNameForGeneratedCertCommonName, }, }, withInformer( @@ -88,16 +90,13 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error { } // Create a CA. - aggregatedAPIServerCA, err := certauthority.New(pkix.Name{CommonName: "Pinniped CA"}, c.certDuration) + aggregatedAPIServerCA, err := certauthority.New(pkix.Name{CommonName: c.generatedCACommonName}, c.certDuration) if err != nil { return fmt.Errorf("could not initialize CA: %w", err) } - // This string must match the name of the Service declared in the deployment yaml. - const serviceName = "pinniped-api" - // Using the CA from above, create a TLS server cert for the aggregated API server to use. - serviceEndpoint := serviceName + "." + c.namespace + ".svc" + serviceEndpoint := c.serviceNameForGeneratedCertCommonName + "." + c.namespace + ".svc" aggregatedAPIServerTLSCert, err := aggregatedAPIServerCA.Issue( pkix.Name{CommonName: serviceEndpoint}, []string{serviceEndpoint}, @@ -129,11 +128,6 @@ func (c *certsManagerController) Sync(ctx controllerlib.Context) error { return fmt.Errorf("could not create secret: %w", err) } - // Update the APIService to give it the new CA bundle. - if err := UpdateAPIService(ctx.Context, c.aggregatorClient, aggregatedAPIServerCA.Bundle()); err != nil { - return fmt.Errorf("could not update the API service: %w", err) - } - - klog.Info("certsManagerController Sync successfully created secret and updated API service") + klog.Info("certsManagerController Sync successfully created secret") return nil } diff --git a/internal/controller/apicerts/certs_manager_test.go b/internal/controller/apicerts/certs_manager_test.go index 0f1d1f5f..fd27c806 100644 --- a/internal/controller/apicerts/certs_manager_test.go +++ b/internal/controller/apicerts/certs_manager_test.go @@ -21,10 +21,7 @@ import ( kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" - apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" - aggregatorfake "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake" - pinnipedv1alpha1 "github.com/suzerain-io/pinniped/generated/1.19/apis/pinniped/v1alpha1" "github.com/suzerain-io/pinniped/internal/controllerlib" "github.com/suzerain-io/pinniped/internal/testutil" ) @@ -43,15 +40,7 @@ func TestManagerControllerOptions(t *testing.T) { observableWithInformerOption = testutil.NewObservableWithInformerOption() observableWithInitialEventOption = testutil.NewObservableWithInitialEventOption() secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets() - _ = NewCertsManagerController( - installedInNamespace, - nil, - nil, - secretsInformer, - observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters - observableWithInitialEventOption.WithInitialEvent, // make it possible to observe the behavior of the initial event - 0, // certDuration, not needed for this test - ) + _ = NewCertsManagerController(installedInNamespace, nil, secretsInformer, observableWithInformerOption.WithInformer, observableWithInitialEventOption.WithInitialEvent, 0, "Pinniped CA", "pinniped-api") secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer) }) @@ -123,7 +112,6 @@ func TestManagerControllerSync(t *testing.T) { var subject controllerlib.Controller var kubeAPIClient *kubernetesfake.Clientset - var aggregatorAPIClient *aggregatorfake.Clientset var kubeInformerClient *kubernetesfake.Clientset var kubeInformers kubeinformers.SharedInformerFactory var timeoutContext context.Context @@ -137,11 +125,12 @@ func TestManagerControllerSync(t *testing.T) { subject = NewCertsManagerController( installedInNamespace, kubeAPIClient, - aggregatorAPIClient, kubeInformers.Core().V1().Secrets(), controllerlib.WithInformer, controllerlib.WithInitialEvent, certDuration, + "Pinniped CA", + "pinniped-api", ) // Set this at the last second to support calling subject.Name(). @@ -167,7 +156,6 @@ func TestManagerControllerSync(t *testing.T) { kubeInformerClient = kubernetesfake.NewSimpleClientset() kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0) kubeAPIClient = kubernetesfake.NewSimpleClientset() - aggregatorAPIClient = aggregatorfake.NewSimpleClientset() }) it.After(func() { @@ -186,111 +174,35 @@ func TestManagerControllerSync(t *testing.T) { r.NoError(err) }) - when("the APIService exists", func() { - it.Before(func() { - apiService := &apiregistrationv1.APIService{ - ObjectMeta: metav1.ObjectMeta{ - Name: pinnipedv1alpha1.SchemeGroupVersion.Version + "." + pinnipedv1alpha1.GroupName, - }, - Spec: apiregistrationv1.APIServiceSpec{ - CABundle: nil, - VersionPriority: 1234, - }, - } - err := aggregatorAPIClient.Tracker().Add(apiService) - r.NoError(err) - }) + it("creates the api-serving-cert Secret", func() { + startInformersAndController() + err := controllerlib.TestSync(t, subject, *syncContext) + r.NoError(err) - it("creates the api-serving-cert Secret and updates the APIService's ca bundle", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.NoError(err) + // Check all the relevant fields from the create Secret action + r.Len(kubeAPIClient.Actions(), 1) + actualAction := kubeAPIClient.Actions()[0].(coretesting.CreateActionImpl) + r.Equal(schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, actualAction.GetResource()) + r.Equal(installedInNamespace, actualAction.GetNamespace()) + actualSecret := actualAction.GetObject().(*corev1.Secret) + r.Equal("api-serving-cert", actualSecret.Name) + r.Equal(installedInNamespace, actualSecret.Namespace) + actualCACert := actualSecret.StringData["caCertificate"] + actualPrivateKey := actualSecret.StringData["tlsPrivateKey"] + actualCertChain := actualSecret.StringData["tlsCertificateChain"] + r.NotEmpty(actualCACert) + r.NotEmpty(actualPrivateKey) + r.NotEmpty(actualCertChain) - // Check all the relevant fields from the create Secret action - r.Len(kubeAPIClient.Actions(), 1) - actualAction := kubeAPIClient.Actions()[0].(coretesting.CreateActionImpl) - r.Equal(schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, actualAction.GetResource()) - r.Equal(installedInNamespace, actualAction.GetNamespace()) - actualSecret := actualAction.GetObject().(*corev1.Secret) - r.Equal("api-serving-cert", actualSecret.Name) - r.Equal(installedInNamespace, actualSecret.Namespace) - actualCACert := actualSecret.StringData["caCertificate"] - actualPrivateKey := actualSecret.StringData["tlsPrivateKey"] - actualCertChain := actualSecret.StringData["tlsCertificateChain"] - r.NotEmpty(actualCACert) - r.NotEmpty(actualPrivateKey) - r.NotEmpty(actualCertChain) + // Validate the created CA's lifetime. + validCACert := testutil.ValidateCertificate(t, actualCACert, actualCACert) + validCACert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute) - // Validate the created CA's lifetime. - validCACert := testutil.ValidateCertificate(t, actualCACert, actualCACert) - validCACert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute) - - // Validate the created cert using the CA, and also validate the cert's hostname - validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain) - validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc") - validCert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute) - validCert.RequireMatchesPrivateKey(actualPrivateKey) - - // Make sure we updated the APIService caBundle and left it otherwise unchanged - r.Len(aggregatorAPIClient.Actions(), 2) - r.Equal("get", aggregatorAPIClient.Actions()[0].GetVerb()) - expectedAPIServiceName := pinnipedv1alpha1.SchemeGroupVersion.Version + "." + pinnipedv1alpha1.GroupName - expectedUpdateAction := coretesting.NewUpdateAction( - schema.GroupVersionResource{ - Group: apiregistrationv1.GroupName, - Version: "v1", - Resource: "apiservices", - }, - "", - &apiregistrationv1.APIService{ - ObjectMeta: metav1.ObjectMeta{ - Name: expectedAPIServiceName, - Namespace: "", - }, - Spec: apiregistrationv1.APIServiceSpec{ - VersionPriority: 1234, // only the CABundle is updated, this other field is left unchanged - CABundle: []byte(actualCACert), - }, - }, - ) - r.Equal(expectedUpdateAction, aggregatorAPIClient.Actions()[1]) - }) - - when("updating the APIService fails", func() { - it.Before(func() { - aggregatorAPIClient.PrependReactor( - "update", - "apiservices", - func(_ coretesting.Action) (bool, runtime.Object, error) { - return true, nil, errors.New("update failed") - }, - ) - }) - - it("returns the update error", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.EqualError(err, "could not update the API service: could not update API service: update failed") - }) - }) - }) - - when("the APIService does not exist", func() { - it.Before(func() { - unrelatedAPIService := &apiregistrationv1.APIService{ - ObjectMeta: metav1.ObjectMeta{Name: "some other api service"}, - Spec: apiregistrationv1.APIServiceSpec{}, - } - err := aggregatorAPIClient.Tracker().Add(unrelatedAPIService) - r.NoError(err) - }) - - it("returns an error", func() { - startInformersAndController() - err := controllerlib.TestSync(t, subject, *syncContext) - r.Error(err) - r.Regexp("could not get existing version of API service: .* not found", err.Error()) - }) + // Validate the created cert using the CA, and also validate the cert's hostname + validCert := testutil.ValidateCertificate(t, actualCACert, actualCertChain) + validCert.RequireDNSName("pinniped-api." + installedInNamespace + ".svc") + validCert.RequireLifetime(time.Now(), time.Now().Add(certDuration), 6*time.Minute) + validCert.RequireMatchesPrivateKey(actualPrivateKey) }) when("creating the Secret fails", func() { @@ -304,11 +216,10 @@ func TestManagerControllerSync(t *testing.T) { ) }) - it("returns the create error and does not update the APIService", func() { + it("returns the create error", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.EqualError(err, "could not create secret: create failed") - r.Empty(aggregatorAPIClient.Actions()) }) }) }) @@ -325,12 +236,11 @@ func TestManagerControllerSync(t *testing.T) { r.NoError(err) }) - it("does not need to make any API calls with its API clients", func() { + it("does not need to make any API calls with its API client", func() { startInformersAndController() err := controllerlib.TestSync(t, subject, *syncContext) r.NoError(err) r.Empty(kubeAPIClient.Actions()) - r.Empty(aggregatorAPIClient.Actions()) }) }) }, spec.Parallel(), spec.Report(report.Terminal{})) diff --git a/internal/controller/apicerts/certs_observer.go b/internal/controller/apicerts/certs_observer.go index a1a5108e..5837f0e5 100644 --- a/internal/controller/apicerts/certs_observer.go +++ b/internal/controller/apicerts/certs_observer.go @@ -54,7 +54,7 @@ func (c *certsObserverController) Sync(_ controllerlib.Context) error { return fmt.Errorf("failed to get %s/%s secret: %w", c.namespace, certsSecretName, err) } if notFound { - klog.Info("certsObserverController Sync() found that the secret does not exist yet or was deleted") + klog.Info("certsObserverController Sync found that the secret does not exist yet or was deleted") // The secret does not exist yet or was deleted. c.dynamicCertProvider.Set(nil, nil) return nil diff --git a/internal/controller/apicerts/update_api_service.go b/internal/controller/apicerts/update_api_service.go index faf6eb55..db39c5c3 100644 --- a/internal/controller/apicerts/update_api_service.go +++ b/internal/controller/apicerts/update_api_service.go @@ -6,6 +6,7 @@ SPDX-License-Identifier: Apache-2.0 package apicerts import ( + "bytes" "context" "fmt" @@ -28,6 +29,11 @@ func UpdateAPIService(ctx context.Context, aggregatorClient aggregatorclient.Int return fmt.Errorf("could not get existing version of API service: %w", err) } + if bytes.Equal(fetchedAPIService.Spec.CABundle, aggregatedAPIServerCA) { + // Already has the same value, perhaps because another process already updated the object, so no need to update. + return nil + } + // Update just the field we care about. fetchedAPIService.Spec.CABundle = aggregatedAPIServerCA diff --git a/internal/controller/apicerts/update_api_service_test.go b/internal/controller/apicerts/update_api_service_test.go index a615a6a8..b9fff892 100644 --- a/internal/controller/apicerts/update_api_service_test.go +++ b/internal/controller/apicerts/update_api_service_test.go @@ -70,17 +70,44 @@ func TestUpdateAPIService(t *testing.T) { }, }}, }, + { + name: "happy path update when the pre-existing APIService already has the same CA bundle so there is no need to update", + mocks: func(c *aggregatorv1fake.Clientset) { + _ = c.Tracker().Add(&apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{Name: apiServiceName}, + Spec: apiregistrationv1.APIServiceSpec{ + GroupPriorityMinimum: 999, + CABundle: []byte("some-ca-bundle"), + }, + }) + c.PrependReactor("update", "apiservices", func(_ kubetesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("should not encounter this error because update should be skipped in this case") + }) + }, + caInput: []byte("some-ca-bundle"), + wantObjects: []apiregistrationv1.APIService{{ + ObjectMeta: metav1.ObjectMeta{Name: apiServiceName}, + Spec: apiregistrationv1.APIServiceSpec{ + GroupPriorityMinimum: 999, + CABundle: []byte("some-ca-bundle"), // unchanged + }, + }}, + }, { name: "error on update", mocks: func(c *aggregatorv1fake.Clientset) { _ = c.Tracker().Add(&apiregistrationv1.APIService{ ObjectMeta: metav1.ObjectMeta{Name: apiServiceName}, - Spec: apiregistrationv1.APIServiceSpec{}, + Spec: apiregistrationv1.APIServiceSpec{ + GroupPriorityMinimum: 999, + CABundle: []byte("some-other-different-ca-bundle"), + }, }) c.PrependReactor("update", "apiservices", func(_ kubetesting.Action) (bool, runtime.Object, error) { return true, nil, fmt.Errorf("error on update") }) }, + caInput: []byte("some-ca-bundle"), wantErr: "could not update API service: error on update", }, { @@ -143,6 +170,7 @@ func TestUpdateAPIService(t *testing.T) { }}, }, } + for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 191cf8f0..dcb565fa 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -47,6 +47,9 @@ func PrepareControllers( kubePublicNamespaceK8sInformers, installationNamespaceK8sInformers, installationNamespacePinnipedInformers := createInformers(serverInstallationNamespace, k8sClient, pinnipedClient) + // This string must match the name of the Service declared in the deployment yaml. + const serviceName = "pinniped-api" + // Create controller manager. controllerManager := controllerlib. NewManager(). @@ -65,11 +68,21 @@ func PrepareControllers( apicerts.NewCertsManagerController( serverInstallationNamespace, k8sClient, - aggregatorClient, installationNamespaceK8sInformers.Core().V1().Secrets(), controllerlib.WithInformer, controllerlib.WithInitialEvent, servingCertDuration, + "Pinniped CA", + serviceName, + ), + singletonWorker, + ). + WithController( + apicerts.NewAPIServiceUpdaterController( + serverInstallationNamespace, + aggregatorClient, + installationNamespaceK8sInformers.Core().V1().Secrets(), + controllerlib.WithInformer, ), singletonWorker, ).