From 20b21e86393440a16109ba071020df191d38f61e Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Tue, 8 Sep 2020 16:36:49 -0700 Subject: [PATCH 01/14] Prefactor: Move updating of APIService to a separate controller - The certs manager controller, along with its sibling certs expirer and certs observer controllers, are generally useful for any process that wants to create its own CA and TLS certs, but only if the updating of the APIService is not included in those controllers - So that functionality for updating APIServices is moved to a new controller which watches the same Secret which is used by those other controllers - Also parameterize `NewCertsManagerController` with the service name and the CA common name to make the controller more reusable --- go.sum | 12 + .../controller/apicerts/apiservice_updater.go | 69 +++++ .../apicerts/apiservice_updater_test.go | 277 ++++++++++++++++++ internal/controller/apicerts/certs_expirer.go | 6 +- internal/controller/apicerts/certs_manager.go | 42 ++- .../controller/apicerts/certs_manager_test.go | 152 ++-------- .../controller/apicerts/certs_observer.go | 2 +- .../controller/apicerts/update_api_service.go | 6 + .../apicerts/update_api_service_test.go | 30 +- .../controllermanager/prepare_controllers.go | 15 +- 10 files changed, 460 insertions(+), 151 deletions(-) create mode 100644 internal/controller/apicerts/apiservice_updater.go create mode 100644 internal/controller/apicerts/apiservice_updater_test.go 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, ). From 3ee7a0d881e1449caa4e2cb35db85552aef093b2 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Sep 2020 15:27:30 -0400 Subject: [PATCH 02/14] cmd/test-webhook: first draft of webhook The webhook still needs to be updated to auto generate its certificates. We decided not to give this webhook its own go module for now since this webhook only pulled in one more dependency, and it is a dependency that we will most likely need in the future. Signed-off-by: Andrew Keesler --- cmd/test-webhook/main.go | 304 ++++++++++++++++++++++ cmd/test-webhook/main_test.go | 457 ++++++++++++++++++++++++++++++++++ go.mod | 1 + 3 files changed, 762 insertions(+) create mode 100644 cmd/test-webhook/main.go create mode 100644 cmd/test-webhook/main_test.go diff --git a/cmd/test-webhook/main.go b/cmd/test-webhook/main.go new file mode 100644 index 00000000..6ed214b4 --- /dev/null +++ b/cmd/test-webhook/main.go @@ -0,0 +1,304 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +// Package main provides a authentication webhook program. +// +// This webhook is meant to be used in demo settings to play around with +// Pinniped. As well, it can come in handy in integration tests. +// +// This webhook is NOT meant for production settings. +package main + +import ( + "bytes" + "context" + "crypto/tls" + "encoding/csv" + "encoding/json" + "fmt" + "net" + "net/http" + "os" + "os/signal" + "strings" + "time" + + "golang.org/x/crypto/bcrypt" + authenticationv1 "k8s.io/api/authentication/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + kubeinformers "k8s.io/client-go/informers" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + restclient "k8s.io/client-go/rest" + "k8s.io/klog/v2" + + "github.com/suzerain-io/pinniped/internal/provider" +) + +const ( + // namespace is the assumed namespace of this webhook. It is hardcoded now for + // simplicity, but should probably be made configurable in the future. + namespace = "test-webhook" + + defaultResyncInterval = 3 * time.Minute +) + +type webhook struct { + certProvider provider.DynamicTLSServingCertProvider + secretInformer corev1informers.SecretInformer +} + +func newWebhook( + certProvider provider.DynamicTLSServingCertProvider, + secretInformer corev1informers.SecretInformer, +) *webhook { + return &webhook{ + certProvider: certProvider, + secretInformer: secretInformer, + } +} + +// start runs the webhook in a separate goroutine and returns whether or not the +// webhook was started successfully. +func (w *webhook) start(ctx context.Context, l net.Listener) error { + server := http.Server{ + Handler: w, + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + GetCertificate: func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { + certPEM, keyPEM := w.certProvider.CurrentCertKeyContent() + cert, err := tls.X509KeyPair(certPEM, keyPEM) + return &cert, err + }, + }, + } + + errCh := make(chan error) + go func() { + // Per ListenAndServeTLS doc, the {cert,key}File parameters can be empty + // since we want to use the certs from http.Server.TLSConfig. + errCh <- server.ServeTLS(l, "", "") + }() + + go func() { + select { + case err := <-errCh: + klog.InfoS("server exited", "err", err) + case <-ctx.Done(): + klog.InfoS("server context cancelled", "err", ctx.Err()) + if err := server.Shutdown(context.Background()); err != nil { + klog.InfoS("server shutdown failed", "err", err) + } + } + }() + + return nil +} + +func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { + defer req.Body.Close() + + if req.URL.Path != "/authenticate" { + rsp.WriteHeader(http.StatusNotFound) + return + } + + if req.Method != http.MethodPost { + rsp.WriteHeader(http.StatusMethodNotAllowed) + return + } + + if !contains(req.Header.Values("Content-Type"), "application/json") { + rsp.WriteHeader(http.StatusUnsupportedMediaType) + return + } + if !contains(req.Header.Values("Accept"), "application/json") { + rsp.WriteHeader(http.StatusUnsupportedMediaType) + return + } + + var body authenticationv1.TokenReview + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + klog.InfoS("failed to decode body", "err", err) + rsp.WriteHeader(http.StatusBadRequest) + return + } + + tokenSegments := strings.SplitN(body.Spec.Token, ":", 2) + if len(tokenSegments) != 2 { + rsp.WriteHeader(http.StatusBadRequest) + return + } + username := tokenSegments[0] + password := tokenSegments[1] + + secret, err := w.secretInformer.Lister().Secrets(namespace).Get(username) + notFound := k8serrors.IsNotFound(err) + if err != nil && !notFound { + klog.InfoS("could not get secret", "err", err) + rsp.WriteHeader(http.StatusInternalServerError) + return + } + + if notFound { + respondWithUnauthenticated(rsp) + return + } + + passwordMatches := bcrypt.CompareHashAndPassword( + secret.Data["passwordHash"], + []byte(password), + ) == nil + if !passwordMatches { + respondWithUnauthenticated(rsp) + } + + groupsBuf := bytes.NewBuffer(secret.Data["groups"]) + gr := csv.NewReader(groupsBuf) + groups, err := gr.Read() + if err != nil { + klog.InfoS("could not read groups", "err", err) + rsp.WriteHeader(http.StatusInternalServerError) + return + } + trimSpace(groups) + + respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups) +} + +func contains(ss []string, s string) bool { + for i := range ss { + if ss[i] == s { + return true + } + } + return false +} + +func trimSpace(ss []string) { + for i := range ss { + ss[i] = strings.TrimSpace(ss[i]) + } +} + +func respondWithUnauthenticated(rsp http.ResponseWriter) { + rsp.Header().Add("Content-Type", "application/json") + + body := authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: false, + }, + } + if err := json.NewEncoder(rsp).Encode(body); err != nil { + klog.InfoS("could not encode response", "err", err) + rsp.WriteHeader(http.StatusInternalServerError) + } +} + +func respondWithAuthenticated( + rsp http.ResponseWriter, + username, uid string, + groups []string, +) { + rsp.Header().Add("Content-Type", "application/json") + + body := authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{ + Username: username, + UID: uid, + Groups: groups, + }, + }, + } + if err := json.NewEncoder(rsp).Encode(body); err != nil { + klog.InfoS("could not encode response", "err", err) + rsp.WriteHeader(http.StatusInternalServerError) + } +} + +func newK8sClient() (kubernetes.Interface, error) { + kubeConfig, err := restclient.InClusterConfig() + if err != nil { + return nil, fmt.Errorf("could not load in-cluster configuration: %w", err) + } + + // Connect to the core Kubernetes API. + kubeClient, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + return nil, fmt.Errorf("could not load in-cluster configuration: %w", err) + } + + return kubeClient, nil +} + +func startControllers(ctx context.Context) error { + return nil +} + +func startWebhook( + ctx context.Context, + l net.Listener, + secretInformer corev1informers.SecretInformer, +) error { + return newWebhook( + provider.NewDynamicTLSServingCertProvider(), + secretInformer, + ).start(ctx, l) +} + +func waitForSignal() os.Signal { + signalCh := make(chan os.Signal, 1) + signal.Notify(signalCh, os.Interrupt) + return <-signalCh +} + +func run() error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + kubeClient, err := newK8sClient() + if err != nil { + return fmt.Errorf("cannot create k8s client: %w", err) + } + + kubeInformers := kubeinformers.NewSharedInformerFactoryWithOptions( + kubeClient, + defaultResyncInterval, + kubeinformers.WithNamespace(namespace), + ) + + if err := startControllers(ctx); err != nil { + return fmt.Errorf("cannot start controllers: %w", err) + } + klog.InfoS("controllers are ready") + + l, err := net.Listen("tcp", "127.0.0.1:443") + if err != nil { + return fmt.Errorf("cannot create listener: %w", err) + } + defer l.Close() + + if err := startWebhook( + ctx, + l, + kubeInformers.Core().V1().Secrets(), + ); err != nil { + return fmt.Errorf("cannot start webhook: %w", err) + } + klog.InfoS("webhook is ready", "address", l.Addr().String()) + + signal := waitForSignal() + klog.InfoS("webhook exiting", "signal", signal) + + return nil +} + +func main() { + if err := run(); err != nil { + klog.Fatal(err) + } +} diff --git a/cmd/test-webhook/main_test.go b/cmd/test-webhook/main_test.go new file mode 100644 index 00000000..82e434c6 --- /dev/null +++ b/cmd/test-webhook/main_test.go @@ -0,0 +1,457 @@ +/* +Copyright 2020 VMware, Inc. +SPDX-License-Identifier: Apache-2.0 +*/ + +package main + +import ( + "bytes" + "context" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net" + "net/http" + "net/url" + "reflect" + "testing" + "time" + + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + kubeinformers "k8s.io/client-go/informers" + corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/client-go/kubernetes" + kubernetesfake "k8s.io/client-go/kubernetes/fake" + + "github.com/suzerain-io/pinniped/internal/certauthority" + "github.com/suzerain-io/pinniped/internal/provider" +) + +func TestWebhook(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + uid, otherUID, colonUID := "some-uid", "some-other-uid", "some-colon-uid" + user, otherUser, colonUser := "some-user", "some-other-user", "some-colon-user" + password, otherPassword, colonPassword := "some-password", "some-other-password", "some-:-password" + group0, group1 := "some-group-0", "some-group-1" + + passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) + require.NoError(t, err) + + otherPasswordHash, err := bcrypt.GenerateFromPassword([]byte(otherPassword), bcrypt.MinCost) + require.NoError(t, err) + + colonPasswordHash, err := bcrypt.GenerateFromPassword([]byte(colonPassword), bcrypt.MinCost) + require.NoError(t, err) + + groups := group0 + " , " + group1 + + userSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(uid), + Name: user, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "passwordHash": passwordHash, + "groups": []byte(groups), + }, + } + otherUserSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(otherUID), + Name: otherUser, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "passwordHash": otherPasswordHash, + "groups": []byte(groups), + }, + } + colonUserSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(colonUID), + Name: colonUser, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "passwordHash": colonPasswordHash, + "groups": []byte(groups), + }, + } + + kubeClient := kubernetesfake.NewSimpleClientset() + require.NoError(t, kubeClient.Tracker().Add(userSecret)) + require.NoError(t, kubeClient.Tracker().Add(otherUserSecret)) + require.NoError(t, kubeClient.Tracker().Add(colonUserSecret)) + + secretInformer := createSecretInformer(t, kubeClient) + + certProvider, caBundle, serverName := newCertProvider(t) + w := newWebhook(certProvider, secretInformer) + + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer l.Close() + require.NoError(t, w.start(ctx, l)) + + client := newClient(caBundle, serverName) + + tests := []struct { + name string + url string + method string + headers map[string][]string + body func() (io.ReadCloser, error) + + wantStatus int + wantHeaders map[string][]string + wantBody *authenticationv1.TokenReview + }{ + { + name: "success", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(user + ":" + password) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{ + Username: user, + UID: uid, + Groups: []string{group0, group1}, + }, + }, + }, + }, + { + name: "wrong username for password", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(otherUser + ":" + password) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: false, + }, + }, + }, + { + name: "wrong password for username", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(user + ":" + otherPassword) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: false, + }, + }, + }, + { + name: "non-existent password for username", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(user + ":" + "some-non-existent-password") + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: false, + }, + }, + }, + { + name: "non-existent username", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody("some-non-existent-user" + ":" + password) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: false, + }, + }, + }, + { + name: "invalid token (missing colon)", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(user) + }, + wantStatus: http.StatusBadRequest, + }, + { + name: "password contains colon", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(colonUser + ":" + colonPassword) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": []string{"application/json"}, + }, + wantBody: &authenticationv1.TokenReview{ + Status: authenticationv1.TokenReviewStatus{ + Authenticated: true, + User: authenticationv1.UserInfo{ + Username: colonUser, + UID: colonUID, + Groups: []string{group0, group1}, + }, + }, + }, + }, + { + name: "bad path", + url: fmt.Sprintf("https://%s/tuna", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody("some-token") + }, + wantStatus: http.StatusNotFound, + }, + { + name: "bad method", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodGet, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody("some-token") + }, + wantStatus: http.StatusMethodNotAllowed, + }, + { + name: "bad content type", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/xml"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody("some-token") + }, + wantStatus: http.StatusUnsupportedMediaType, + }, + { + name: "bad accept", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/xml"}, + }, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody("some-token") + }, + wantStatus: http.StatusUnsupportedMediaType, + }, + { + name: "bad body", + url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + }, + body: func() (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewBuffer([]byte("invalid body"))), nil + }, + wantStatus: http.StatusBadRequest, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + url, err := url.Parse(test.url) + require.NoError(t, err) + + body, err := test.body() + require.NoError(t, err) + + rsp, err := client.Do(&http.Request{ + Method: test.method, + URL: url, + Header: test.headers, + Body: body, + }) + require.NoError(t, err) + defer rsp.Body.Close() + + if test.wantStatus != 0 { + require.Equal(t, test.wantStatus, rsp.StatusCode) + } + if test.wantHeaders != nil { + for k, v := range test.wantHeaders { + require.Equal(t, v, rsp.Header.Values(k)) + } + } + if test.wantBody != nil { + rspBody, err := newTokenReview(rsp.Body) + require.NoError(t, err) + require.Equal(t, test.wantBody, rspBody) + } + }) + } +} + +func createSecretInformer(t *testing.T, kubeClient kubernetes.Interface) corev1informers.SecretInformer { + t.Helper() + + kubeInformers := kubeinformers.NewSharedInformerFactory(kubeClient, 0) + + secretInformer := kubeInformers.Core().V1().Secrets() + + // We need to call Informer() on the secretInformer to lazily instantiate the + // informer factory before syncing it. + secretInformer.Informer() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) + defer cancel() + + kubeInformers.Start(ctx.Done()) + + informerTypesSynced := kubeInformers.WaitForCacheSync(ctx.Done()) + require.True(t, informerTypesSynced[reflect.TypeOf(&corev1.Secret{})]) + + return secretInformer +} + +// newClientProvider returns a provider.DynamicTLSServingCertProvider configured +// with valid serving cert, the CA bundle that can be used to verify the serving +// cert, and the server name that can be used to verify the TLS peer. +func newCertProvider(t *testing.T) (provider.DynamicTLSServingCertProvider, []byte, string) { + t.Helper() + + ca, err := certauthority.New(pkix.Name{CommonName: "test-webhook CA"}, time.Hour*24) + require.NoError(t, err) + + serverName := "test-webhook" + cert, err := ca.Issue( + pkix.Name{CommonName: serverName}, + []string{}, + time.Hour*24, + ) + require.NoError(t, err) + + certPEM, keyPEM, err := certauthority.ToPEM(cert) + require.NoError(t, err) + + certProvider := provider.NewDynamicTLSServingCertProvider() + certProvider.Set(certPEM, keyPEM) + + return certProvider, ca.Bundle(), serverName +} + +// newClient creates an http.Client that can be used to make an HTTPS call to a +// service whose serving certs can be verified by the provided CA bundle. +func newClient(caBundle []byte, serverName string) *http.Client { + rootCAs := x509.NewCertPool() + rootCAs.AppendCertsFromPEM(caBundle) + return &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + RootCAs: rootCAs, + ServerName: serverName, + }, + }, + } +} + +// newTokenReviewBody creates an io.ReadCloser that contains a JSON-encoded +// TokenReview request. +func newTokenReviewBody(token string) (io.ReadCloser, error) { + buf := bytes.NewBuffer([]byte{}) + tr := authenticationv1.TokenReview{ + Spec: authenticationv1.TokenReviewSpec{ + Token: token, + }, + } + err := json.NewEncoder(buf).Encode(&tr) + return ioutil.NopCloser(buf), err +} + +// newTokenReview reads a JSON-encoded authenticationv1.TokenReview from an +// io.Reader. +func newTokenReview(body io.Reader) (*authenticationv1.TokenReview, error) { + var tr authenticationv1.TokenReview + err := json.NewDecoder(body).Decode(&tr) + return &tr, err +} diff --git a/go.mod b/go.mod index 5c6f75b2..246020d6 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/stretchr/testify v1.6.1 github.com/suzerain-io/pinniped/generated/1.19/apis v0.0.0-00010101000000-000000000000 github.com/suzerain-io/pinniped/generated/1.19/client v0.0.0-00010101000000-000000000000 + golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 k8s.io/api v0.19.0 k8s.io/apimachinery v0.19.0 k8s.io/apiserver v0.19.0 From 2565f67824b72e0356b410308923cf8e1caa7687 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 9 Sep 2020 19:06:39 -0700 Subject: [PATCH 03/14] Create a deployment for `test-webhook` - For now, build the test-webhook binary in the same container image as the pinniped-server binary, to make it easier to distribute - Also fix lots of bugs from the first draft of the test-webhook's `/authenticate` implementation from the previous commit - Add a detailed README for the new deploy-test-webhook directory --- Dockerfile | 7 +- cmd/test-webhook/main.go | 133 +++++---- cmd/test-webhook/main_test.go | 420 ++++++++++++++++------------ deploy-test-webhook/README.md | 110 ++++++++ deploy-test-webhook/deployment.yaml | 63 +++++ deploy-test-webhook/rbac.yaml | 30 ++ deploy-test-webhook/values.yaml | 10 + deploy/README.md | 7 + 8 files changed, 542 insertions(+), 238 deletions(-) create mode 100644 deploy-test-webhook/README.md create mode 100644 deploy-test-webhook/deployment.yaml create mode 100644 deploy-test-webhook/rbac.yaml create mode 100644 deploy-test-webhook/values.yaml diff --git a/Dockerfile b/Dockerfile index d0f4f03e..eba31050 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,13 +19,16 @@ COPY tools ./tools COPY hack ./hack # Build the executable binary (CGO_ENABLED=0 means static linking) -RUN mkdir out && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags "$(hack/get-ldflags.sh)" -o out ./cmd/pinniped-server/... +RUN mkdir out \ + && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags "$(hack/get-ldflags.sh)" -o out ./cmd/pinniped-server/... \ + && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o out ./cmd/test-webhook/... # Use a runtime image based on Debian slim FROM debian:10.5-slim -# Copy the binary from the build-env stage +# Copy the binaries from the build-env stage COPY --from=build-env /work/out/pinniped-server /usr/local/bin/pinniped-server +COPY --from=build-env /work/out/test-webhook /usr/local/bin/test-webhook # Document the port EXPOSE 443 diff --git a/cmd/test-webhook/main.go b/cmd/test-webhook/main.go index 6ed214b4..b74809db 100644 --- a/cmd/test-webhook/main.go +++ b/cmd/test-webhook/main.go @@ -8,7 +8,7 @@ SPDX-License-Identifier: Apache-2.0 // This webhook is meant to be used in demo settings to play around with // Pinniped. As well, it can come in handy in integration tests. // -// This webhook is NOT meant for production settings. +// This webhook is NOT meant for use in production systems. package main import ( @@ -34,14 +34,24 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/klog/v2" + "github.com/suzerain-io/pinniped/internal/controller/apicerts" + "github.com/suzerain-io/pinniped/internal/controllerlib" "github.com/suzerain-io/pinniped/internal/provider" ) const ( - // namespace is the assumed namespace of this webhook. It is hardcoded now for - // simplicity, but should probably be made configurable in the future. + // This string must match the name of the Namespace declared in the deployment yaml. namespace = "test-webhook" + // This string must match the name of the Service declared in the deployment yaml. + serviceName = "test-webhook" + // TODO there must be a better way to get this specific json result string without needing to hardcode it + unauthenticatedResponse = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":false}}` + + // TODO there must be a better way to get this specific json result string without needing to hardcode it + authenticatedResponseTemplate = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":true,"user":{"username":"%s","uid":"%s","groups":%s}}}` + + singletonWorker = 1 defaultResyncInterval = 3 * time.Minute ) @@ -153,17 +163,21 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { ) == nil if !passwordMatches { respondWithUnauthenticated(rsp) - } - - groupsBuf := bytes.NewBuffer(secret.Data["groups"]) - gr := csv.NewReader(groupsBuf) - groups, err := gr.Read() - if err != nil { - klog.InfoS("could not read groups", "err", err) - rsp.WriteHeader(http.StatusInternalServerError) return } - trimSpace(groups) + + groups := []string{} + groupsBuf := bytes.NewBuffer(secret.Data["groups"]) + if groupsBuf.Len() > 0 { + groupsCSVReader := csv.NewReader(groupsBuf) + groups, err = groupsCSVReader.Read() + if err != nil { + klog.InfoS("could not read groups", "err", err) + rsp.WriteHeader(http.StatusInternalServerError) + return + } + trimLeadingAndTrailingWhitespace(groups) + } respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups) } @@ -177,7 +191,7 @@ func contains(ss []string, s string) bool { return false } -func trimSpace(ss []string) { +func trimLeadingAndTrailingWhitespace(ss []string) { for i := range ss { ss[i] = strings.TrimSpace(ss[i]) } @@ -185,16 +199,7 @@ func trimSpace(ss []string) { func respondWithUnauthenticated(rsp http.ResponseWriter) { rsp.Header().Add("Content-Type", "application/json") - - body := authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: false, - }, - } - if err := json.NewEncoder(rsp).Encode(body); err != nil { - klog.InfoS("could not encode response", "err", err) - rsp.WriteHeader(http.StatusInternalServerError) - } + _, _ = rsp.Write([]byte(unauthenticatedResponse)) } func respondWithAuthenticated( @@ -203,21 +208,14 @@ func respondWithAuthenticated( groups []string, ) { rsp.Header().Add("Content-Type", "application/json") - - body := authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: true, - User: authenticationv1.UserInfo{ - Username: username, - UID: uid, - Groups: groups, - }, - }, - } - if err := json.NewEncoder(rsp).Encode(body); err != nil { + groupsJSONBytes, err := json.Marshal(groups) + if err != nil { klog.InfoS("could not encode response", "err", err) rsp.WriteHeader(http.StatusInternalServerError) + return } + jsonBody := fmt.Sprintf(authenticatedResponseTemplate, username, uid, groupsJSONBytes) + _, _ = rsp.Write([]byte(jsonBody)) } func newK8sClient() (kubernetes.Interface, error) { @@ -235,19 +233,52 @@ func newK8sClient() (kubernetes.Interface, error) { return kubeClient, nil } -func startControllers(ctx context.Context) error { - return nil +func startControllers( + ctx context.Context, + dynamicCertProvider provider.DynamicTLSServingCertProvider, + kubeClient kubernetes.Interface, + kubeInformers kubeinformers.SharedInformerFactory, +) { + aVeryLongTime := time.Hour * 24 * 365 * 100 + + // Create controller manager. + controllerManager := controllerlib. + NewManager(). + WithController( + apicerts.NewCertsManagerController( + namespace, + kubeClient, + kubeInformers.Core().V1().Secrets(), + controllerlib.WithInformer, + controllerlib.WithInitialEvent, + aVeryLongTime, + "test-webhook CA", + serviceName, + ), + singletonWorker, + ). + WithController( + apicerts.NewCertsObserverController( + namespace, + dynamicCertProvider, + kubeInformers.Core().V1().Secrets(), + controllerlib.WithInformer, + ), + singletonWorker, + ) + + kubeInformers.Start(ctx.Done()) + + go controllerManager.Start(ctx) } func startWebhook( ctx context.Context, l net.Listener, + dynamicCertProvider provider.DynamicTLSServingCertProvider, secretInformer corev1informers.SecretInformer, ) error { - return newWebhook( - provider.NewDynamicTLSServingCertProvider(), - secretInformer, - ).start(ctx, l) + return newWebhook(dynamicCertProvider, secretInformer).start(ctx, l) } func waitForSignal() os.Signal { @@ -271,28 +302,26 @@ func run() error { kubeinformers.WithNamespace(namespace), ) - if err := startControllers(ctx); err != nil { - return fmt.Errorf("cannot start controllers: %w", err) - } + dynamicCertProvider := provider.NewDynamicTLSServingCertProvider() + + startControllers(ctx, dynamicCertProvider, kubeClient, kubeInformers) klog.InfoS("controllers are ready") - l, err := net.Listen("tcp", "127.0.0.1:443") + //nolint: gosec + l, err := net.Listen("tcp", ":443") if err != nil { return fmt.Errorf("cannot create listener: %w", err) } defer l.Close() - if err := startWebhook( - ctx, - l, - kubeInformers.Core().V1().Secrets(), - ); err != nil { + err = startWebhook(ctx, l, dynamicCertProvider, kubeInformers.Core().V1().Secrets()) + if err != nil { return fmt.Errorf("cannot start webhook: %w", err) } klog.InfoS("webhook is ready", "address", l.Addr().String()) - signal := waitForSignal() - klog.InfoS("webhook exiting", "signal", signal) + gotSignal := waitForSignal() + klog.InfoS("webhook exiting", "signal", gotSignal) return nil } diff --git a/cmd/test-webhook/main_test.go b/cmd/test-webhook/main_test.go index 82e434c6..699bc24d 100644 --- a/cmd/test-webhook/main_test.go +++ b/cmd/test-webhook/main_test.go @@ -19,6 +19,7 @@ import ( "net/http" "net/url" "reflect" + "strings" "testing" "time" @@ -43,60 +44,48 @@ func TestWebhook(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - uid, otherUID, colonUID := "some-uid", "some-other-uid", "some-colon-uid" - user, otherUser, colonUser := "some-user", "some-other-user", "some-colon-user" - password, otherPassword, colonPassword := "some-password", "some-other-password", "some-:-password" + user, otherUser, colonUser, noGroupUser, oneGroupUser, passwordUndefinedUser, emptyPasswordUser, underfinedGroupsUser := + "some-user", "other-user", "colon-user", "no-group-user", "one-group-user", "password-undefined-user", "empty-password-user", "undefined-groups-user" + uid, otherUID, colonUID, noGroupUID, oneGroupUID, passwordUndefinedUID, emptyPasswordUID, underfinedGroupsUID := + "some-uid", "other-uid", "colon-uid", "no-group-uid", "one-group-uid", "password-undefined-uid", "empty-password-uid", "undefined-groups-uid" + password, otherPassword, colonPassword, noGroupPassword, oneGroupPassword, undefinedGroupsPassword := + "some-password", "other-password", "some-:-password", "no-group-password", "one-group-password", "undefined-groups-password" + group0, group1 := "some-group-0", "some-group-1" - - passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) - require.NoError(t, err) - - otherPasswordHash, err := bcrypt.GenerateFromPassword([]byte(otherPassword), bcrypt.MinCost) - require.NoError(t, err) - - colonPasswordHash, err := bcrypt.GenerateFromPassword([]byte(colonPassword), bcrypt.MinCost) - require.NoError(t, err) - groups := group0 + " , " + group1 - userSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(uid), - Name: user, - Namespace: "test-webhook", - }, - Data: map[string][]byte{ - "passwordHash": passwordHash, - "groups": []byte(groups), - }, - } - otherUserSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(otherUID), - Name: otherUser, - Namespace: "test-webhook", - }, - Data: map[string][]byte{ - "passwordHash": otherPasswordHash, - "groups": []byte(groups), - }, - } - colonUserSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - UID: types.UID(colonUID), - Name: colonUser, - Namespace: "test-webhook", - }, - Data: map[string][]byte{ - "passwordHash": colonPasswordHash, - "groups": []byte(groups), - }, - } - kubeClient := kubernetesfake.NewSimpleClientset() - require.NoError(t, kubeClient.Tracker().Add(userSecret)) - require.NoError(t, kubeClient.Tracker().Add(otherUserSecret)) - require.NoError(t, kubeClient.Tracker().Add(colonUserSecret)) + addSecretToFakeClientTracker(t, kubeClient, user, uid, password, groups) + addSecretToFakeClientTracker(t, kubeClient, otherUser, otherUID, otherPassword, groups) + addSecretToFakeClientTracker(t, kubeClient, colonUser, colonUID, colonPassword, groups) + addSecretToFakeClientTracker(t, kubeClient, noGroupUser, noGroupUID, noGroupPassword, "") + addSecretToFakeClientTracker(t, kubeClient, oneGroupUser, oneGroupUID, oneGroupPassword, group0) + addSecretToFakeClientTracker(t, kubeClient, emptyPasswordUser, emptyPasswordUID, "", groups) + + require.NoError(t, kubeClient.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(passwordUndefinedUID), + Name: passwordUndefinedUser, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "groups": []byte(groups), + }, + })) + + undefinedGroupsUserPasswordHash, err := bcrypt.GenerateFromPassword([]byte(undefinedGroupsPassword), bcrypt.MinCost) + require.NoError(t, err) + + require.NoError(t, kubeClient.Tracker().Add(&corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(underfinedGroupsUID), + Name: underfinedGroupsUser, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "passwordHash": undefinedGroupsUserPasswordHash, + }, + })) secretInformer := createSecretInformer(t, kubeClient) @@ -110,6 +99,12 @@ func TestWebhook(t *testing.T) { client := newClient(caBundle, serverName) + goodURL := fmt.Sprintf("https://%s/authenticate", l.Addr().String()) + goodRequestHeaders := map[string][]string{ + "Content-Type": {"application/json"}, + "Accept": {"application/json"}, + } + tests := []struct { name string url string @@ -119,178 +114,187 @@ func TestWebhook(t *testing.T) { wantStatus int wantHeaders map[string][]string - wantBody *authenticationv1.TokenReview + wantBody *string }{ { - name: "success", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "success for a user who belongs to multiple groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: true, - User: authenticationv1.UserInfo{ - Username: user, - UID: uid, - Groups: []string{group0, group1}, - }, - }, + "Content-Type": {"application/json"}, }, + wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), }, { - name: "wrong username for password", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, + name: "success for a user who belongs to one groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(oneGroupUser + ":" + oneGroupPassword) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": {"application/json"}, + }, + wantBody: authenticatedResponseJSON(oneGroupUser, oneGroupUID, []string{group0}), + }, + { + name: "success for a user who belongs to no groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(noGroupUser + ":" + noGroupPassword) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": {"application/json"}, + }, + wantBody: authenticatedResponseJSON(noGroupUser, noGroupUID, []string{}), + }, + { + name: "wrong username for password", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(otherUser + ":" + password) }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: false, - }, + "Content-Type": {"application/json"}, }, + wantBody: unauthenticatedResponseJSON(), }, { - name: "wrong password for username", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, + name: "when a user has no password hash in the secret", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(passwordUndefinedUser + ":foo") }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": {"application/json"}, + }, + wantBody: unauthenticatedResponseJSON(), + }, + { + name: "success for a user has no groups defined in the secret", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(underfinedGroupsUser + ":" + undefinedGroupsPassword) + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": {"application/json"}, + }, + wantBody: authenticatedResponseJSON(underfinedGroupsUser, underfinedGroupsUID, []string{}), + }, + { + name: "when a user has empty string as their password", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { + return newTokenReviewBody(passwordUndefinedUser + ":foo") + }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{ + "Content-Type": {"application/json"}, + }, + wantBody: unauthenticatedResponseJSON(), + }, + { + name: "wrong password for username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + otherPassword) }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: false, - }, + "Content-Type": {"application/json"}, }, + wantBody: unauthenticatedResponseJSON(), }, { - name: "non-existent password for username", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "non-existent password for username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + "some-non-existent-password") }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: false, - }, + "Content-Type": {"application/json"}, }, + wantBody: unauthenticatedResponseJSON(), }, { - name: "non-existent username", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "non-existent username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-non-existent-user" + ":" + password) }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: false, - }, + "Content-Type": {"application/json"}, }, + wantBody: unauthenticatedResponseJSON(), }, { - name: "invalid token (missing colon)", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "bad token format (missing colon)", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(user) }, wantStatus: http.StatusBadRequest, }, { - name: "password contains colon", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "password contains colon", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody(colonUser + ":" + colonPassword) }, wantStatus: http.StatusOK, wantHeaders: map[string][]string{ - "Content-Type": []string{"application/json"}, - }, - wantBody: &authenticationv1.TokenReview{ - Status: authenticationv1.TokenReviewStatus{ - Authenticated: true, - User: authenticationv1.UserInfo{ - Username: colonUser, - UID: colonUID, - Groups: []string{group0, group1}, - }, - }, + "Content-Type": {"application/json"}, }, + wantBody: authenticatedResponseJSON(colonUser, colonUID, []string{group0, group1}), }, { - name: "bad path", - url: fmt.Sprintf("https://%s/tuna", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "bad path", + url: fmt.Sprintf("https://%s/tuna", l.Addr().String()), + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, wantStatus: http.StatusNotFound, }, { - name: "bad method", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodGet, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "bad method", + url: goodURL, + method: http.MethodGet, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, @@ -298,11 +302,11 @@ func TestWebhook(t *testing.T) { }, { name: "bad content type", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + url: goodURL, method: http.MethodPost, headers: map[string][]string{ - "Content-Type": []string{"application/xml"}, - "Accept": []string{"application/json"}, + "Content-Type": {"application/xml"}, + "Accept": {"application/json"}, }, body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") @@ -311,11 +315,11 @@ func TestWebhook(t *testing.T) { }, { name: "bad accept", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), + url: goodURL, method: http.MethodPost, headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/xml"}, + "Content-Type": {"application/json"}, + "Accept": {"application/xml"}, }, body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") @@ -323,23 +327,21 @@ func TestWebhook(t *testing.T) { wantStatus: http.StatusUnsupportedMediaType, }, { - name: "bad body", - url: fmt.Sprintf("https://%s/authenticate", l.Addr().String()), - method: http.MethodPost, - headers: map[string][]string{ - "Content-Type": []string{"application/json"}, - "Accept": []string{"application/json"}, - }, + name: "bad body", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, body: func() (io.ReadCloser, error) { return ioutil.NopCloser(bytes.NewBuffer([]byte("invalid body"))), nil }, wantStatus: http.StatusBadRequest, }, } + for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - url, err := url.Parse(test.url) + parsedURL, err := url.Parse(test.url) require.NoError(t, err) body, err := test.body() @@ -347,25 +349,28 @@ func TestWebhook(t *testing.T) { rsp, err := client.Do(&http.Request{ Method: test.method, - URL: url, + URL: parsedURL, Header: test.headers, Body: body, }) require.NoError(t, err) defer rsp.Body.Close() - if test.wantStatus != 0 { - require.Equal(t, test.wantStatus, rsp.StatusCode) - } + require.Equal(t, test.wantStatus, rsp.StatusCode) + if test.wantHeaders != nil { for k, v := range test.wantHeaders { require.Equal(t, v, rsp.Header.Values(k)) } } + + responseBody, err := ioutil.ReadAll(rsp.Body) + require.NoError(t, err) if test.wantBody != nil { - rspBody, err := newTokenReview(rsp.Body) require.NoError(t, err) - require.Equal(t, test.wantBody, rspBody) + require.JSONEq(t, *test.wantBody, string(responseBody)) + } else { + require.Empty(t, responseBody) } }) } @@ -448,10 +453,57 @@ func newTokenReviewBody(token string) (io.ReadCloser, error) { return ioutil.NopCloser(buf), err } -// newTokenReview reads a JSON-encoded authenticationv1.TokenReview from an -// io.Reader. -func newTokenReview(body io.Reader) (*authenticationv1.TokenReview, error) { - var tr authenticationv1.TokenReview - err := json.NewDecoder(body).Decode(&tr) - return &tr, err +func unauthenticatedResponseJSON() *string { + // Very specific expected result. Avoid using json package so we know exactly what we're asserting. + s := `{ + "apiVersion": "authentication.k8s.io/v1beta1", + "kind": "TokenReview", + "status": { + "authenticated": false + } + }` + return &s +} + +func authenticatedResponseJSON(user, uid string, groups []string) *string { + var quotedGroups []string + for _, group := range groups { + quotedGroups = append(quotedGroups, `"`+group+`"`) + } + + // Very specific expected result. Avoid using json package so we know exactly what we're asserting. + authenticatedJSONTemplate := `{ + "apiVersion": "authentication.k8s.io/v1beta1", + "kind": "TokenReview", + "status": { + "authenticated": true, + "user": { + "username": "%s", + "uid": "%s", + "groups": [%s] + } + } + }` + + s := fmt.Sprintf(authenticatedJSONTemplate, user, uid, strings.Join(quotedGroups, ",")) + return &s +} + +func addSecretToFakeClientTracker(t *testing.T, kubeClient *kubernetesfake.Clientset, username, uid, password, groups string) { + passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(uid), + Name: username, + Namespace: "test-webhook", + }, + Data: map[string][]byte{ + "passwordHash": passwordHash, + "groups": []byte(groups), + }, + } + + require.NoError(t, kubeClient.Tracker().Add(secret)) } diff --git a/deploy-test-webhook/README.md b/deploy-test-webhook/README.md new file mode 100644 index 00000000..f802fece --- /dev/null +++ b/deploy-test-webhook/README.md @@ -0,0 +1,110 @@ +# Deploying `test-webhook` + +## What is `test-webhook`? + +The `test-webhook` app is an identity provider used for integration testing and demos. +If you would like to demo Pinniped, but you don't have a compatible identity provider handy, +you can use Pinniped's `test-webhook` identity provider. Note that this is not recommended for +production use. + +The `test-webhook` is a Kubernetes Deployment which runs a webhook server that implements the Kubernetes +[Webhook Token Authentication interface](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#webhook-token-authentication). + +User accounts can be created and edited dynamically using `kubectl` commands (see below). + +## Tools + +This example deployment uses `ytt` from [Carvel](https://carvel.dev/) to template the YAML files. +Either [install `ytt`](https://get-ytt.io/) or use the [container image from Dockerhub](https://hub.docker.com/r/k14s/image/tags). + +## Procedure + +1. The configuration options are in [values.yml](values.yaml). Fill in the values in that file, or override those values + using `ytt` command-line options in the command below. +2. In a terminal, cd to this `deploy-test-webhook` directory +3. To generate the final YAML files, run: `ytt --file .` +4. Deploy the generated YAML using your preferred deployment tool, such as `kubectl` or [`kapp`](https://get-kapp.io/). + For example: `ytt --file . | kapp deploy --yes --app test-webhook --diff-changes --file -` + +## Configuring After Installing + +### Create Users + +Use `kubectl` to create, edit, and delete user accounts by creating a `Secret` for each user account in the same +namespace where `test-webhook` is deployed. The name of the `Secret` resource is the username. +Store the user's group membership and `bcrypt` encrypted password as the contents of the `Secret`. +For example, to create a user named `ryan` with the password `password123` +who belongs to the groups `group1` and `group2`, use: + +```bash +kubectl create secret generic ryan \ + --namespace test-webhook \ + --from-literal=groups=group1,group2 \ + --from-literal=passwordHash=$(htpasswd -nbBC 16 x password123 | sed -e "s/^x://") +``` + +### Get the `test-webhook` App's Auto-Generated Certificate Authority Bundle + +Fetch the auto-generated CA bundle for the `test-webhook`'s HTTP TLS endpoint. + +```bash +kubectl get secret api-serving-cert --namespace test-webhook \ + -o jsonpath={.data.caCertificate} \ + | base64 -d \ + | tee /tmp/test-webhook-ca +``` + +### Configuring Pinniped to Use `test-webhook` as an Identity Provider + +When installing Pinniped on the same cluster, configure `test-webhook` as an Identity Provider for Pinniped +using the webhook URL `https://test-webhook.test-webhook.svc/authenticate` +along with the CA bundle fetched by the above command. + +### Optional: Manually Test the Webhook Endpoint + + 1. Start a pod from which you can curl the endpoint from inside the cluster. + + ```bash + kubectl run curlpod --image=curlimages/curl --command -- /bin/sh -c "while true; do echo hi; sleep 120; done" + ``` + + 1. Copy the CA bundle that was fetched above onto the new pod. + + ```bash + kubectl cp /tmp/test-webhook-ca curlpod:/tmp/test-webhook-ca + ``` + + 1. Run a `curl` command to try to authenticate as the user created above. + + ```bash + kubectl -it exec curlpod -- curl https://test-webhook.test-webhook.svc/authenticate \ + --cacert /tmp/test-webhook-ca \ + -H 'Content-Type: application/json' -H 'Accept: application/json' -d ' + { + "apiVersion": "authentication.k8s.io/v1beta1", + "kind": "TokenReview", + "spec": { + "token": "ryan:password123" + } + }' + ``` + + When authentication is successful the above command should return some JSON similar to the following. + Note that the value of `authenticated` is `true` to indicate a successful authentication. + + ```json + {"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":true,"user":{"username":"ryan","uid":"19c433ec-8f58-44ca-9ef0-2d1081ccb876","groups":["group1","group2"]}}} + ``` + + Trying the above `curl` command again with the wrong username or password in the body of the request + should result in a JSON response which indicates that the authentication failed. + + ```json + {"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":false}} + ``` + + 1. Remove the curl pod. + + ```bash + kubectl delete pod curlpod + ``` diff --git a/deploy-test-webhook/deployment.yaml b/deploy-test-webhook/deployment.yaml new file mode 100644 index 00000000..b9a794a2 --- /dev/null +++ b/deploy-test-webhook/deployment.yaml @@ -0,0 +1,63 @@ +#! Copyright 2020 VMware, Inc. +#! SPDX-License-Identifier: Apache-2.0 + +#@ load("@ytt:data", "data") + +--- +apiVersion: v1 +kind: Namespace +metadata: + name: test-webhook + labels: + name: test-webhook +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: test-webhook-service-account + namespace: test-webhook +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-webhook + namespace: test-webhook + labels: + app: test-webhook +spec: + replicas: 1 + selector: + matchLabels: + app: test-webhook + template: + metadata: + labels: + app: test-webhook + spec: + serviceAccountName: test-webhook-service-account + containers: + - name: test-webhook + #@ if data.values.image_digest: + image: #@ data.values.image_repo + "@" + data.values.image_digest + #@ else: + image: #@ data.values.image_repo + ":" + data.values.image_tag + #@ end + imagePullPolicy: IfNotPresent + command: #! override the default entrypoint + - /usr/local/bin/test-webhook +--- +apiVersion: v1 +kind: Service +metadata: + name: test-webhook + namespace: test-webhook + labels: + app: test-webhook +spec: + type: ClusterIP + selector: + app: test-webhook + ports: + - protocol: TCP + port: 443 + targetPort: 443 diff --git a/deploy-test-webhook/rbac.yaml b/deploy-test-webhook/rbac.yaml new file mode 100644 index 00000000..a111c5a1 --- /dev/null +++ b/deploy-test-webhook/rbac.yaml @@ -0,0 +1,30 @@ +#! Copyright 2020 VMware, Inc. +#! SPDX-License-Identifier: Apache-2.0 + +#@ load("@ytt:data", "data") + +#! Give permission to various objects within the app's own namespace +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: test-webhook-aggregated-api-server-role + namespace: test-webhook +rules: + - apiGroups: [""] + resources: [secrets] + verbs: [create, get, list, patch, update, watch] +--- +kind: RoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: test-webhook-aggregated-api-server-role-binding + namespace: test-webhook +subjects: + - kind: ServiceAccount + name: test-webhook-service-account + namespace: test-webhook +roleRef: + kind: Role + name: test-webhook-aggregated-api-server-role + apiGroup: rbac.authorization.k8s.io diff --git a/deploy-test-webhook/values.yaml b/deploy-test-webhook/values.yaml new file mode 100644 index 00000000..d8531a2c --- /dev/null +++ b/deploy-test-webhook/values.yaml @@ -0,0 +1,10 @@ +#! Copyright 2020 VMware, Inc. +#! SPDX-License-Identifier: Apache-2.0 + +#@data/values +--- + +#! Specify either an image_digest or an image_tag. If both are given, only image_digest will be used. +image_repo: #! e.g. registry.example.com/your-project-name/repo-name +image_digest: #! e.g. sha256:f3c4fdfd3ef865d4b97a1fd295d94acc3f0c654c46b6f27ffad5cf80216903c8 +image_tag: #! e.g. latest diff --git a/deploy/README.md b/deploy/README.md index 1eb0eb17..2e6cf752 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -1,5 +1,12 @@ # Deploying +## Connecting Pinniped to an Identity Provider + +If you would like to try Pinniped, but you don't have a compatible identity provider, +you can use Pinniped's test identity provider. +See [../deplot-test-webhook/README.md](../deplot-test-webhook/README.md) +for details. + ## Tools This example deployment uses `ytt` from [Carvel](https://carvel.dev/) to template the YAML files. From 89d01b84f87741d7b863c9d9dfa0e75e23189a9d Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 10 Sep 2020 09:33:46 -0400 Subject: [PATCH 04/14] deploy/README.md: fix markdown link to test webhook README.md Signed-off-by: Andrew Keesler --- deploy/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/README.md b/deploy/README.md index 2e6cf752..ba16679f 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -4,7 +4,7 @@ If you would like to try Pinniped, but you don't have a compatible identity provider, you can use Pinniped's test identity provider. -See [../deplot-test-webhook/README.md](../deplot-test-webhook/README.md) +See [../deploy-test-webhook/README.md](../deploy-test-webhook/README.md) for details. ## Tools From fec31b71c0395a58751615f062d0efe0c97fac9f Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 10 Sep 2020 09:50:17 -0400 Subject: [PATCH 05/14] deploy-test-webhook/README.md: add another tool needed for the demo The other diffs in this comment were dictated by pre-commit. Signed-off-by: Andrew Keesler --- deploy-test-webhook/README.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/deploy-test-webhook/README.md b/deploy-test-webhook/README.md index f802fece..61626a1f 100644 --- a/deploy-test-webhook/README.md +++ b/deploy-test-webhook/README.md @@ -17,6 +17,11 @@ User accounts can be created and edited dynamically using `kubectl` commands (se This example deployment uses `ytt` from [Carvel](https://carvel.dev/) to template the YAML files. Either [install `ytt`](https://get-ytt.io/) or use the [container image from Dockerhub](https://hub.docker.com/r/k14s/image/tags). +As well, this demo requires a tool capable of generating a `bcrypt` hash in order to interact with +the webhook. The example below uses `htpasswd`, which is installed on most macOS systems, and can be +installed on some Linux systems via the `apache2-utils` package (e.g., `apt-get install +apache2-utils`). + ## Procedure 1. The configuration options are in [values.yml](values.yaml). Fill in the values in that file, or override those values @@ -30,10 +35,10 @@ Either [install `ytt`](https://get-ytt.io/) or use the [container image from Doc ### Create Users -Use `kubectl` to create, edit, and delete user accounts by creating a `Secret` for each user account in the same +Use `kubectl` to create, edit, and delete user accounts by creating a `Secret` for each user account in the same namespace where `test-webhook` is deployed. The name of the `Secret` resource is the username. Store the user's group membership and `bcrypt` encrypted password as the contents of the `Secret`. -For example, to create a user named `ryan` with the password `password123` +For example, to create a user named `ryan` with the password `password123` who belongs to the groups `group1` and `group2`, use: ```bash @@ -96,7 +101,7 @@ along with the CA bundle fetched by the above command. {"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":true,"user":{"username":"ryan","uid":"19c433ec-8f58-44ca-9ef0-2d1081ccb876","groups":["group1","group2"]}}} ``` - Trying the above `curl` command again with the wrong username or password in the body of the request + Trying the above `curl` command again with the wrong username or password in the body of the request should result in a JSON response which indicates that the authentication failed. ```json @@ -104,7 +109,7 @@ along with the CA bundle fetched by the above command. ``` 1. Remove the curl pod. - + ```bash kubectl delete pod curlpod ``` From b506ac5823758b7e6d27a6f952fc91307f18cf1d Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 10 Sep 2020 11:30:15 -0400 Subject: [PATCH 06/14] Port integration test setup script from CI repo I also started updating the script to deploy the test-webhook instead of doing TMC stuff. I think the script should live in this repo so that Pinniped contributors only need to worry about one repo for running integration tests. There are a bunch of TODOs in the script, but I figured this was a good checkpoint. The script successfully runs on my machine and sets up the test-webhook and pinniped on a local kind cluster. The integration tests are failing because of some issue with pinniped talking to the test-webhook, but this is step in the right direction. Signed-off-by: Andrew Keesler --- hack/prepare-for-integration-tests.sh | 316 +++++++++++++++++++++ test/integration/client_test.go | 4 +- test/integration/credentialrequest_test.go | 70 +++-- 3 files changed, 359 insertions(+), 31 deletions(-) create mode 100755 hack/prepare-for-integration-tests.sh diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh new file mode 100755 index 00000000..1842e798 --- /dev/null +++ b/hack/prepare-for-integration-tests.sh @@ -0,0 +1,316 @@ +#!/usr/bin/env bash + +# This script can be used to prepare a kind cluster and deploy the app. +# You can call this script again to redeploy the app. +# It will also output instructions on how to run the integration or uninstall tests. + +# TODO: get rid of references to ci repo +# TODO: fix uninstall test setup +# TODO: add flag to let user provide registry/tag for their image +# \- (this can be used by kind integration tests from kind-load-and-docker-run-any-script.sh) +# TODO: add flag to setup current-context for integration tests + +set -euo pipefail + +function print_or_redact_doc() { + doc_kind=$(echo "$1" | awk '/^kind: / {print $2}') + if [[ -z "$doc_kind" ]]; then + echo "warning: " + elif [[ $doc_kind == "Secret" || $doc_kind == "secret" ]]; then + echo + echo "---" + echo "" + else + printf "%s\n" "$1" + fi +} + +function print_redacted_manifest() { + doc="" + while IFS="" read -r line || [ -n "$line" ]; do + if [[ $line == "---" ]]; then + if [[ -n "$doc" ]]; then + print_or_redact_doc "$doc" + fi + doc="" + fi + doc=$(printf "%s\n%s" "$doc" "$line") + done <"$1" + + print_or_redact_doc "$doc" +} + +function log_note() { + GREEN='\033[0;32m' + NC='\033[0m' + if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then + echo -e " ${GREEN}:)${NC} Note: $*" + else + echo " :) Note: $*" + fi +} + +function log_warning() { + YELLOW='\033[0;33m' + NC='\033[0m' + if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then + echo -e " ${YELLOW}:/${NC} Warning: $*" + else + echo " :/ Warning: $*" + fi +} + +function log_error() { + RED='\033[0;31m' + NC='\033[0m' + if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then + echo -e " ${RED}:(${NC} Error: $*" + else + echo " :( Error: $*" + fi +} + +help=no +skip_build=no +prepare_for_uninstall_test=no + +PARAMS="" +while (("$#")); do + case "$1" in + -h | --help) + help=yes + shift + ;; + -s | --skip-build) + skip_build=yes + shift + ;; + -u | --prepare-uninstall) + prepare_for_uninstall_test=yes + shift + ;; + -*) + log_error "Unsupported flag $1" >&2 + exit 1 + ;; + *) + PARAMS="$PARAMS $1" + shift + ;; + esac +done +eval set -- "$PARAMS" + +if [[ "$help" == "yes" ]]; then + me="$(basename "${BASH_SOURCE[0]}")" + echo "Usage:" + echo " $me [flags] [path/to/pinniped] [path/to/pinniped-ci]" + echo + echo " path/to/pinniped default: \$PWD ($PWD)" + echo + echo "Flags:" + echo " -h, --help: print this usage" + echo " -s, --skip-build: reuse the most recently built image of the app instead of building" + echo " -u, --prepare-uninstall: delete the kind cluster and prepare to run the install+uninstall test" + exit 1 +fi + +pinniped_path="${1-$PWD}" + +if ! command -v kind >/dev/null; then + log_error "Please install kind. e.g. 'brew install kind' for MacOS" + exit 1 +fi + +if ! command -v ytt >/dev/null; then + log_error "Please install ytt. e.g. 'brew tap k14s/tap && brew install ytt' for MacOS" + exit 1 +fi + +if ! command -v kapp >/dev/null; then + log_error "Please install kapp. e.g. 'brew tap k14s/tap && brew install kapp' for MacOS" + exit 1 +fi + +if ! command -v kubectl >/dev/null; then + log_error "Please install kubectl. e.g. 'brew install kubectl' for MacOS" + exit 1 +fi + +cd "$pinniped_path" || exit 1 + +if [[ ! -f Dockerfile || ! -d deploy ]]; then + log_error "$pinniped_path does not appear to be the path to the source code repo directory" + exit 1 +fi + +if [[ "$prepare_for_uninstall_test" == "yes" ]]; then + log_note "Deleting running kind clusters to prepare a clean slate for the install+uninstall test..." + kind delete cluster +fi + +log_note "Checking for running kind clusters..." +if ! kind get clusters | grep -q -e '^kind$'; then + log_note "Creating a kind cluster..." + kind create cluster +else + if ! kubectl cluster-info | grep master | grep -q 127.0.0.1; then + log_error "Seems like your kubeconfig is not targeting a local cluster." + log_error "Exiting to avoid accidentally running tests against a real cluster." + exit 1 + fi +fi + +registry="docker.io" +repo="test/build" +registry_repo="$registry/$repo" +tag=$(uuidgen) # always a new tag to force K8s to reload the image on redeploy + +if [[ "$skip_build" == "yes" ]]; then + most_recent_tag=$(docker images "$repo" --format "{{.Tag}}" | head -1) + if [[ -n "$most_recent_tag" ]]; then + tag="$most_recent_tag" + do_build=no + else + # Oops, there was no previous build. Need to build anyway. + do_build=yes + fi +else + do_build=yes +fi + +registry_repo_tag="${registry_repo}:${tag}" + +if [[ "$do_build" == "yes" ]]; then + # Rebuild the code + log_note "Docker building the app..." + docker build . --tag "$registry_repo_tag" +fi + +# Load it into the cluster +log_note "Loading the app's container image into the kind cluster..." +kind load docker-image "$registry_repo_tag" + +if [[ "$prepare_for_uninstall_test" == "yes" ]]; then + cat </tmp/uninstall-test-env +# The following env vars should be set before running $pinniped_ci_path/pipelines/shared-tasks/run-uninstall-test/run-uninstall-test.sh +export IMAGE_REPO="$registry_repo" +export IMAGE_TAG="$tag" +export PINNIPED_DISCOVERY_URL="$discovery_url" +EOF + + log_note "Done!" + log_note + log_note "Ready to run the uninstall test." + log_note " cd $pinniped_path" + log_note ' source /tmp/uninstall-test-env' + log_note " $pinniped_ci_path/pipelines/shared-tasks/run-uninstall-test/run-uninstall-test.sh" + log_note + log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." + +else + manifest=/tmp/manifest.yaml + + # + # Deploy test-webhook + # + pushd deploy-test-webhook >/dev/null + + log_note "Deploying the test-webhook app to the cluster..." + ytt --file . \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" >"$manifest" + + echo + log_note "Full test-webhook app manifest with Secrets redacted..." + echo "--------------------------------------------------------------------------------" + print_redacted_manifest $manifest + echo "--------------------------------------------------------------------------------" + echo + + kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. + kapp deploy --yes --app test-webhook --diff-changes --file "$manifest" + + popd >/dev/null + + log_note "Creating test user 'test-username'..." + test_username="test-username" + test_password="test-password" + test_groups="test-group-0,test-group-1" + kubectl create secret generic "$test_username" \ + --namespace test-webhook \ + --from-literal=groups="$test_groups" \ + --from-literal=passwordHash=$(htpasswd -nbBC 16 x "$test_password" | sed -e "s/^x://") \ + --dry-run=client \ + --output yaml \ + | kubectl apply -f - + + app_name="pinniped" + namespace="integration" + webhook_url="test-webhook.test-webhook.svc" + webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace test-webhook \ + -o jsonpath={.data.caCertificate})" + discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" + + # + # Deploy Pinniped + # + pushd deploy >/dev/null + + log_note "Deploying the Pinniped app to the cluster..." + ytt --file . \ + --data-value "app_name=$app_name" \ + --data-value "namespace=$namespace" \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" \ + --data-value "webhook_url=$webhook_url" \ + --data-value "webhook_ca_bundle=$webhook_ca_bundle" \ + --data-value "discovery_url=$discovery_url" >"$manifest" + + echo + log_note "Full Pinniped app manifest with Secrets redacted..." + echo "--------------------------------------------------------------------------------" + print_redacted_manifest $manifest + echo "--------------------------------------------------------------------------------" + echo + + kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. + kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" + + popd >/dev/null + + kind_capabilities_file="$pinniped_path/test/cluster_capabilities/kind.yaml" + pinniped_cluster_capability_file_content=$(cat "$kind_capabilities_file") + + cat </tmp/integration-test-env +# The following env vars should be set before running 'cd test && go test ./...' +export PINNIPED_NAMESPACE=${namespace} +export PINNIPED_APP_NAME=${app_name} +export PINNIPED_CREDENTIAL_REQUEST_TOKEN=${test_username}:${test_password} + +read -r -d '' PINNIPED_CLUSTER_CAPABILITY_YAML << PINNIPED_CLUSTER_CAPABILITY_YAML_EOF || true +${pinniped_cluster_capability_file_content} +PINNIPED_CLUSTER_CAPABILITY_YAML_EOF + +export PINNIPED_CLUSTER_CAPABILITY_YAML +EOF + + log_note "Done!" + log_note + log_note "Ready to run integration tests. For example, you could run all tests using the following commands..." + log_note " cd $pinniped_path" + log_note ' source /tmp/integration-test-env' + log_note ' (cd test && go test -count 1 ./...)' + log_note + log_note '"Environment" setting for GoLand run configurations:' + log_note -n ' ' + goland_vars=$(grep -v '^#' /tmp/integration-test-env | grep -E '^export .+=' | sed 's/export //g' | tr '\n' ';') + log_note "${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" + log_note + log_note + log_note "You can run this script again to deploy local production code changes while you are working." + log_note + log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." + log_note + log_note "To delete the deployments, run 'kapp delete -a test-webhook -y && kapp delete -a pinniped -y'." +fi diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 38d476ae..da8e202a 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -57,7 +57,7 @@ var maskKey = func(s string) string { return strings.ReplaceAll(s, "TESTING KEY" func TestClient(t *testing.T) { library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - tmcClusterToken := library.GetEnv(t, "PINNIPED_TMC_CLUSTER_TOKEN") + token := library.GetEnv(t, "PINNIPED_CREDENTIAL_REQUEST_TOKEN") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -69,7 +69,7 @@ func TestClient(t *testing.T) { // Using the CA bundle and host from the current (admin) kubeconfig, do the token exchange. clientConfig := library.NewClientConfig(t) - resp, err := client.ExchangeToken(ctx, tmcClusterToken, string(clientConfig.CAData), clientConfig.Host) + resp, err := client.ExchangeToken(ctx, token, string(clientConfig.CAData), clientConfig.Host) require.NoError(t, err) require.NotNil(t, resp.Status.ExpirationTimestamp) require.InDelta(t, time.Until(resp.Status.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute)) diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index 1251a03e..5dce437b 100644 --- a/test/integration/credentialrequest_test.go +++ b/test/integration/credentialrequest_test.go @@ -85,34 +85,36 @@ func TestSuccessfulCredentialRequest(t *testing.T) { require.NotEmpty(t, listNamespaceResponse.Items) }) - t.Run("access as group", func(t *testing.T) { - addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "integration-test-group-readonly-role-binding", - }, - Subjects: []rbacv1.Subject{{ - Kind: rbacv1.GroupKind, - APIGroup: rbacv1.GroupName, - Name: "tmc:member", - }}, - RoleRef: rbacv1.RoleRef{ - Kind: "ClusterRole", - APIGroup: rbacv1.GroupName, - Name: "view", - }, - }) + for _, group := range getOrganizationalUnits(t, response.Status.Credential.ClientCertificateData) { + t.Run("access as group "+group, func(t *testing.T) { + addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "integration-test-group-readonly-role-binding", + }, + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.GroupKind, + APIGroup: rbacv1.GroupName, + Name: group, + }}, + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + APIGroup: rbacv1.GroupName, + Name: "view", + }, + }) - // Use the client which is authenticated as the TMC group to list namespaces - var listNamespaceResponse *v1.NamespaceList - var canListNamespaces = func() bool { - listNamespaceResponse, err = clientWithCertFromCredentialRequest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) - return err == nil - } - assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond) - require.NoError(t, err) // prints out the error and stops the test in case of failure - require.NotEmpty(t, listNamespaceResponse.Items) - }) + // Use the client which is authenticated as the TMC group to list namespaces + var listNamespaceResponse *v1.NamespaceList + var canListNamespaces = func() bool { + listNamespaceResponse, err = clientWithCertFromCredentialRequest.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + return err == nil + } + assert.Eventually(t, canListNamespaces, 3*time.Second, 250*time.Millisecond) + require.NoError(t, err) // prints out the error and stops the test in case of failure + require.NotEmpty(t, listNamespaceResponse.Items) + }) + } } func TestFailedCredentialRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTheUser(t *testing.T) { @@ -183,11 +185,11 @@ func makeRequest(t *testing.T, spec v1alpha1.CredentialRequestSpec) (*v1alpha1.C } func validCredentialRequestSpecWithRealToken(t *testing.T) v1alpha1.CredentialRequestSpec { - tmcClusterToken := library.GetEnv(t, "PINNIPED_TMC_CLUSTER_TOKEN") + token := library.GetEnv(t, "PINNIPED_CREDENTIAL_REQUEST_TOKEN") return v1alpha1.CredentialRequestSpec{ Type: v1alpha1.TokenCredentialType, - Token: &v1alpha1.CredentialRequestTokenCredential{Value: tmcClusterToken}, + Token: &v1alpha1.CredentialRequestTokenCredential{Value: token}, } } @@ -224,3 +226,13 @@ func getCommonName(t *testing.T, certPEM string) string { return cert.Subject.CommonName } + +func getOrganizationalUnits(t *testing.T, certPEM string) []string { + t.Helper() + + pemBlock, _ := pem.Decode([]byte(certPEM)) + cert, err := x509.ParseCertificate(pemBlock.Bytes) + require.NoError(t, err) + + return cert.Subject.OrganizationalUnit +} From 56be4a6761fd337c95290f8cf55f5c826699731a Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 10 Sep 2020 13:37:25 -0700 Subject: [PATCH 07/14] Add more logging to test-webhook's endpoint - Also correct the webhook url setting in prepare-for-integration-tests.sh - Change the bcrypt count to 10, because 16 is way too slow on old laptops Signed-off-by: Ryan Richard --- cmd/test-webhook/main.go | 8 ++++++++ deploy-test-webhook/README.md | 2 +- hack/prepare-for-integration-tests.sh | 26 +++++++++++++------------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/cmd/test-webhook/main.go b/cmd/test-webhook/main.go index b74809db..8c3622bd 100644 --- a/cmd/test-webhook/main.go +++ b/cmd/test-webhook/main.go @@ -111,20 +111,24 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { defer req.Body.Close() if req.URL.Path != "/authenticate" { + klog.InfoS("received request path other than /authenticate", "path", req.URL.Path) rsp.WriteHeader(http.StatusNotFound) return } if req.Method != http.MethodPost { + klog.InfoS("received request method other than post", "method", req.Method) rsp.WriteHeader(http.StatusMethodNotAllowed) return } if !contains(req.Header.Values("Content-Type"), "application/json") { + klog.InfoS("wrong content type", "Content-Type", req.Header.Values("Content-Type")) rsp.WriteHeader(http.StatusUnsupportedMediaType) return } if !contains(req.Header.Values("Accept"), "application/json") { + klog.InfoS("wrong accept type", "Accept", req.Header.Values("Accept")) rsp.WriteHeader(http.StatusUnsupportedMediaType) return } @@ -138,6 +142,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { tokenSegments := strings.SplitN(body.Spec.Token, ":", 2) if len(tokenSegments) != 2 { + klog.InfoS("bad token format in request") rsp.WriteHeader(http.StatusBadRequest) return } @@ -153,6 +158,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { } if notFound { + klog.InfoS("user not found") respondWithUnauthenticated(rsp) return } @@ -162,6 +168,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { []byte(password), ) == nil if !passwordMatches { + klog.InfoS("invalid password in request") respondWithUnauthenticated(rsp) return } @@ -179,6 +186,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { trimLeadingAndTrailingWhitespace(groups) } + klog.InfoS("successful authentication") respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups) } diff --git a/deploy-test-webhook/README.md b/deploy-test-webhook/README.md index 61626a1f..6aaac5bf 100644 --- a/deploy-test-webhook/README.md +++ b/deploy-test-webhook/README.md @@ -45,7 +45,7 @@ who belongs to the groups `group1` and `group2`, use: kubectl create secret generic ryan \ --namespace test-webhook \ --from-literal=groups=group1,group2 \ - --from-literal=passwordHash=$(htpasswd -nbBC 16 x password123 | sed -e "s/^x://") + --from-literal=passwordHash=$(htpasswd -nbBC 10 x password123 | sed -e "s/^x://") ``` ### Get the `test-webhook` App's Auto-Generated Certificate Authority Bundle diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 1842e798..db885112 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -44,9 +44,9 @@ function log_note() { GREEN='\033[0;32m' NC='\033[0m' if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then - echo -e " ${GREEN}:)${NC} Note: $*" + echo -e "${GREEN}$*${NC}" else - echo " :) Note: $*" + echo "$*" fi } @@ -54,9 +54,9 @@ function log_warning() { YELLOW='\033[0;33m' NC='\033[0m' if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then - echo -e " ${YELLOW}:/${NC} Warning: $*" + echo -e "😒${YELLOW} Warning: $* ${NC}" else - echo " :/ Warning: $*" + echo ":/ Warning: $*" fi } @@ -64,9 +64,9 @@ function log_error() { RED='\033[0;31m' NC='\033[0m' if [[ $COLORTERM =~ ^(truecolor|24bit)$ ]]; then - echo -e " ${RED}:(${NC} Error: $*" + echo -e "🙁${RED} Error: $* ${NC}" else - echo " :( Error: $*" + echo ":( Error: $*" fi } @@ -235,21 +235,21 @@ else log_note "Creating test user 'test-username'..." test_username="test-username" + # TODO AUTO-GENERATE PASSWORD test_password="test-password" test_groups="test-group-0,test-group-1" kubectl create secret generic "$test_username" \ --namespace test-webhook \ --from-literal=groups="$test_groups" \ - --from-literal=passwordHash=$(htpasswd -nbBC 16 x "$test_password" | sed -e "s/^x://") \ + --from-literal=passwordHash="$(htpasswd -nbBC 10 x "$test_password" | sed -e "s/^x://")" \ --dry-run=client \ --output yaml \ | kubectl apply -f - app_name="pinniped" namespace="integration" - webhook_url="test-webhook.test-webhook.svc" - webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace test-webhook \ - -o jsonpath={.data.caCertificate})" + webhook_url="https://test-webhook.test-webhook.svc/authenticate" + webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace test-webhook -o 'jsonpath={.data.caCertificate}')" discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" # @@ -295,6 +295,8 @@ PINNIPED_CLUSTER_CAPABILITY_YAML_EOF export PINNIPED_CLUSTER_CAPABILITY_YAML EOF + goland_vars=$(grep -v '^#' /tmp/integration-test-env | grep -E '^export .+=' | sed 's/export //g' | tr '\n' ';') + log_note "Done!" log_note log_note "Ready to run integration tests. For example, you could run all tests using the following commands..." @@ -303,9 +305,7 @@ EOF log_note ' (cd test && go test -count 1 ./...)' log_note log_note '"Environment" setting for GoLand run configurations:' - log_note -n ' ' - goland_vars=$(grep -v '^#' /tmp/integration-test-env | grep -E '^export .+=' | sed 's/export //g' | tr '\n' ';') - log_note "${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" + log_note " ${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" log_note log_note log_note "You can run this script again to deploy local production code changes while you are working." From 9baea830660ad4c0e472e851e3c85b88228603e4 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 10 Sep 2020 15:00:53 -0700 Subject: [PATCH 08/14] Improve the parsing of headers in test-webhook Signed-off-by: Andrew Keesler --- cmd/test-webhook/main.go | 24 ++- cmd/test-webhook/main_test.go | 305 ++++++++++++++++------------------ 2 files changed, 161 insertions(+), 168 deletions(-) diff --git a/cmd/test-webhook/main.go b/cmd/test-webhook/main.go index 8c3622bd..9214ad0d 100644 --- a/cmd/test-webhook/main.go +++ b/cmd/test-webhook/main.go @@ -18,6 +18,7 @@ import ( "encoding/csv" "encoding/json" "fmt" + "mime" "net" "net/http" "os" @@ -122,13 +123,15 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { return } - if !contains(req.Header.Values("Content-Type"), "application/json") { - klog.InfoS("wrong content type", "Content-Type", req.Header.Values("Content-Type")) + if !headerContains(req, "Content-Type", "application/json") { + klog.InfoS("content type is not application/json", "Content-Type", req.Header.Values("Content-Type")) rsp.WriteHeader(http.StatusUnsupportedMediaType) return } - if !contains(req.Header.Values("Accept"), "application/json") { - klog.InfoS("wrong accept type", "Accept", req.Header.Values("Accept")) + if !headerContains(req, "Accept", "application/json") && + !headerContains(req, "Accept", "application/*") && + !headerContains(req, "Accept", "*/*") { + klog.InfoS("client does not accept application/json", "Accept", req.Header.Values("Accept")) rsp.WriteHeader(http.StatusUnsupportedMediaType) return } @@ -190,10 +193,15 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups) } -func contains(ss []string, s string) bool { - for i := range ss { - if ss[i] == s { - return true +func headerContains(req *http.Request, headerName, s string) bool { + headerValues := req.Header.Values(headerName) + for i := range headerValues { + mimeTypes := strings.Split(headerValues[i], ",") + for _, mimeType := range mimeTypes { + mediaType, _, _ := mime.ParseMediaType(mimeType) + if mediaType == s { + return true + } } } return false diff --git a/cmd/test-webhook/main_test.go b/cmd/test-webhook/main_test.go index 699bc24d..66de0aa0 100644 --- a/cmd/test-webhook/main_test.go +++ b/cmd/test-webhook/main_test.go @@ -101,8 +101,8 @@ func TestWebhook(t *testing.T) { goodURL := fmt.Sprintf("https://%s/authenticate", l.Addr().String()) goodRequestHeaders := map[string][]string{ - "Content-Type": {"application/json"}, - "Accept": {"application/json"}, + "Content-Type": {"application/json; charset=UTF-8"}, + "Accept": {"application/json, */*"}, } tests := []struct { @@ -117,74 +117,54 @@ func TestWebhook(t *testing.T) { wantBody *string }{ { - name: "success for a user who belongs to multiple groups", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(user + ":" + password) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), + name: "success for a user who belongs to multiple groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), }, { - name: "success for a user who belongs to one groups", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(oneGroupUser + ":" + oneGroupPassword) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: authenticatedResponseJSON(oneGroupUser, oneGroupUID, []string{group0}), + name: "success for a user who belongs to one groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(oneGroupUser + ":" + oneGroupPassword) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(oneGroupUser, oneGroupUID, []string{group0}), }, { - name: "success for a user who belongs to no groups", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(noGroupUser + ":" + noGroupPassword) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: authenticatedResponseJSON(noGroupUser, noGroupUID, []string{}), + name: "success for a user who belongs to no groups", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(noGroupUser + ":" + noGroupPassword) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(noGroupUser, noGroupUID, []string{}), }, { - name: "wrong username for password", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(otherUser + ":" + password) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "wrong username for password", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(otherUser + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { - name: "when a user has no password hash in the secret", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(passwordUndefinedUser + ":foo") - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "when a user has no password hash in the secret", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(passwordUndefinedUser + ":foo") }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { name: "success for a user has no groups defined in the secret", @@ -194,110 +174,82 @@ func TestWebhook(t *testing.T) { body: func() (io.ReadCloser, error) { return newTokenReviewBody(underfinedGroupsUser + ":" + undefinedGroupsPassword) }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: authenticatedResponseJSON(underfinedGroupsUser, underfinedGroupsUID, []string{}), + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(underfinedGroupsUser, underfinedGroupsUID, []string{}), }, { - name: "when a user has empty string as their password", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(passwordUndefinedUser + ":foo") - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "when a user has empty string as their password", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(passwordUndefinedUser + ":foo") }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { - name: "wrong password for username", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(user + ":" + otherPassword) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "wrong password for username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + otherPassword) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { - name: "non-existent password for username", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(user + ":" + "some-non-existent-password") - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "non-existent password for username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + "some-non-existent-password") }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { - name: "non-existent username", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody("some-non-existent-user" + ":" + password) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: unauthenticatedResponseJSON(), + name: "non-existent username", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-non-existent-user" + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: unauthenticatedResponseJSON(), }, { - name: "bad token format (missing colon)", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(user) - }, + name: "bad token format (missing colon)", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user) }, wantStatus: http.StatusBadRequest, }, { - name: "password contains colon", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody(colonUser + ":" + colonPassword) - }, - wantStatus: http.StatusOK, - wantHeaders: map[string][]string{ - "Content-Type": {"application/json"}, - }, - wantBody: authenticatedResponseJSON(colonUser, colonUID, []string{group0, group1}), + name: "password contains colon", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(colonUser + ":" + colonPassword) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(colonUser, colonUID, []string{group0, group1}), }, { - name: "bad path", - url: fmt.Sprintf("https://%s/tuna", l.Addr().String()), - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody("some-token") - }, + name: "bad path", + url: fmt.Sprintf("https://%s/tuna", l.Addr().String()), + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, wantStatus: http.StatusNotFound, }, { - name: "bad method", - url: goodURL, - method: http.MethodGet, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody("some-token") - }, + name: "bad method", + url: goodURL, + method: http.MethodGet, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, wantStatus: http.StatusMethodNotAllowed, }, { @@ -308,9 +260,7 @@ func TestWebhook(t *testing.T) { "Content-Type": {"application/xml"}, "Accept": {"application/json"}, }, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody("some-token") - }, + body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, wantStatus: http.StatusUnsupportedMediaType, }, { @@ -321,19 +271,54 @@ func TestWebhook(t *testing.T) { "Content-Type": {"application/json"}, "Accept": {"application/xml"}, }, - body: func() (io.ReadCloser, error) { - return newTokenReviewBody("some-token") - }, + body: func() (io.ReadCloser, error) { return newTokenReviewBody("some-token") }, wantStatus: http.StatusUnsupportedMediaType, }, { - name: "bad body", - url: goodURL, - method: http.MethodPost, - headers: goodRequestHeaders, - body: func() (io.ReadCloser, error) { - return ioutil.NopCloser(bytes.NewBuffer([]byte("invalid body"))), nil + name: "success when there are multiple accepts and one of them is json", + url: goodURL, + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": {"application/json"}, + "Accept": {"something/else, application/xml, application/json"}, }, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), + }, + { + name: "success when there are multiple accepts and one of them is */*", + url: goodURL, + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": {"application/json"}, + "Accept": {"something/else, */*, application/foo"}, + }, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), + }, + { + name: "success when there are multiple accepts and one of them is application/*", + url: goodURL, + method: http.MethodPost, + headers: map[string][]string{ + "Content-Type": {"application/json"}, + "Accept": {"something/else, application/*, application/foo"}, + }, + body: func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) }, + wantStatus: http.StatusOK, + wantHeaders: map[string][]string{"Content-Type": {"application/json"}}, + wantBody: authenticatedResponseJSON(user, uid, []string{group0, group1}), + }, + { + name: "bad body", + url: goodURL, + method: http.MethodPost, + headers: goodRequestHeaders, + body: func() (io.ReadCloser, error) { return ioutil.NopCloser(bytes.NewBuffer([]byte("invalid body"))), nil }, wantStatus: http.StatusBadRequest, }, } From b7bdb7f3b163e78fa44b1ea7e54aada940d5cdb0 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 10 Sep 2020 15:20:02 -0700 Subject: [PATCH 09/14] Rename `test-webhook` to `local-user-authenticator` Signed-off-by: Andrew Keesler --- Dockerfile | 4 +- .../main.go | 6 +-- .../main_test.go | 10 ++--- .../README.md | 38 +++++++++---------- .../deployment.yaml | 32 ++++++++-------- .../rbac.yaml | 14 +++---- .../values.yaml | 0 deploy/README.md | 2 +- hack/prepare-for-integration-tests.sh | 18 ++++----- 9 files changed, 62 insertions(+), 62 deletions(-) rename cmd/{test-webhook => local-user-authenticator}/main.go (98%) rename cmd/{test-webhook => local-user-authenticator}/main_test.go (98%) rename {deploy-test-webhook => deploy-local-user-authenticator}/README.md (68%) rename {deploy-test-webhook => deploy-local-user-authenticator}/deployment.yaml (57%) rename {deploy-test-webhook => deploy-local-user-authenticator}/rbac.yaml (58%) rename {deploy-test-webhook => deploy-local-user-authenticator}/values.yaml (100%) diff --git a/Dockerfile b/Dockerfile index eba31050..8992730b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,14 +21,14 @@ COPY hack ./hack # Build the executable binary (CGO_ENABLED=0 means static linking) RUN mkdir out \ && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags "$(hack/get-ldflags.sh)" -o out ./cmd/pinniped-server/... \ - && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o out ./cmd/test-webhook/... + && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o out ./cmd/local-user-authenticator/... # Use a runtime image based on Debian slim FROM debian:10.5-slim # Copy the binaries from the build-env stage COPY --from=build-env /work/out/pinniped-server /usr/local/bin/pinniped-server -COPY --from=build-env /work/out/test-webhook /usr/local/bin/test-webhook +COPY --from=build-env /work/out/local-user-authenticator /usr/local/bin/local-user-authenticator # Document the port EXPOSE 443 diff --git a/cmd/test-webhook/main.go b/cmd/local-user-authenticator/main.go similarity index 98% rename from cmd/test-webhook/main.go rename to cmd/local-user-authenticator/main.go index 9214ad0d..b5d1400e 100644 --- a/cmd/test-webhook/main.go +++ b/cmd/local-user-authenticator/main.go @@ -42,9 +42,9 @@ import ( const ( // This string must match the name of the Namespace declared in the deployment yaml. - namespace = "test-webhook" + namespace = "local-user-authenticator" // This string must match the name of the Service declared in the deployment yaml. - serviceName = "test-webhook" + serviceName = "local-user-authenticator" // TODO there must be a better way to get this specific json result string without needing to hardcode it unauthenticatedResponse = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":false}}` @@ -268,7 +268,7 @@ func startControllers( controllerlib.WithInformer, controllerlib.WithInitialEvent, aVeryLongTime, - "test-webhook CA", + "local-user-authenticator CA", serviceName, ), singletonWorker, diff --git a/cmd/test-webhook/main_test.go b/cmd/local-user-authenticator/main_test.go similarity index 98% rename from cmd/test-webhook/main_test.go rename to cmd/local-user-authenticator/main_test.go index 66de0aa0..4ac44fa6 100644 --- a/cmd/test-webhook/main_test.go +++ b/cmd/local-user-authenticator/main_test.go @@ -66,7 +66,7 @@ func TestWebhook(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ UID: types.UID(passwordUndefinedUID), Name: passwordUndefinedUser, - Namespace: "test-webhook", + Namespace: "local-user-authenticator", }, Data: map[string][]byte{ "groups": []byte(groups), @@ -80,7 +80,7 @@ func TestWebhook(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ UID: types.UID(underfinedGroupsUID), Name: underfinedGroupsUser, - Namespace: "test-webhook", + Namespace: "local-user-authenticator", }, Data: map[string][]byte{ "passwordHash": undefinedGroupsUserPasswordHash, @@ -389,10 +389,10 @@ func createSecretInformer(t *testing.T, kubeClient kubernetes.Interface) corev1i func newCertProvider(t *testing.T) (provider.DynamicTLSServingCertProvider, []byte, string) { t.Helper() - ca, err := certauthority.New(pkix.Name{CommonName: "test-webhook CA"}, time.Hour*24) + ca, err := certauthority.New(pkix.Name{CommonName: "local-user-authenticator CA"}, time.Hour*24) require.NoError(t, err) - serverName := "test-webhook" + serverName := "local-user-authenticator" cert, err := ca.Issue( pkix.Name{CommonName: serverName}, []string{}, @@ -482,7 +482,7 @@ func addSecretToFakeClientTracker(t *testing.T, kubeClient *kubernetesfake.Clien ObjectMeta: metav1.ObjectMeta{ UID: types.UID(uid), Name: username, - Namespace: "test-webhook", + Namespace: "local-user-authenticator", }, Data: map[string][]byte{ "passwordHash": passwordHash, diff --git a/deploy-test-webhook/README.md b/deploy-local-user-authenticator/README.md similarity index 68% rename from deploy-test-webhook/README.md rename to deploy-local-user-authenticator/README.md index 6aaac5bf..a4f4b88f 100644 --- a/deploy-test-webhook/README.md +++ b/deploy-local-user-authenticator/README.md @@ -1,13 +1,13 @@ -# Deploying `test-webhook` +# Deploying `local-user-authenticator` -## What is `test-webhook`? +## What is `local-user-authenticator`? -The `test-webhook` app is an identity provider used for integration testing and demos. +The `local-user-authenticator` app is an identity provider used for integration testing and demos. If you would like to demo Pinniped, but you don't have a compatible identity provider handy, -you can use Pinniped's `test-webhook` identity provider. Note that this is not recommended for +you can use Pinniped's `local-user-authenticator` identity provider. Note that this is not recommended for production use. -The `test-webhook` is a Kubernetes Deployment which runs a webhook server that implements the Kubernetes +The `local-user-authenticator` is a Kubernetes Deployment which runs a webhook server that implements the Kubernetes [Webhook Token Authentication interface](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#webhook-token-authentication). User accounts can be created and edited dynamically using `kubectl` commands (see below). @@ -26,43 +26,43 @@ apache2-utils`). 1. The configuration options are in [values.yml](values.yaml). Fill in the values in that file, or override those values using `ytt` command-line options in the command below. -2. In a terminal, cd to this `deploy-test-webhook` directory +2. In a terminal, cd to this `deploy-local-user-authenticator` directory 3. To generate the final YAML files, run: `ytt --file .` 4. Deploy the generated YAML using your preferred deployment tool, such as `kubectl` or [`kapp`](https://get-kapp.io/). - For example: `ytt --file . | kapp deploy --yes --app test-webhook --diff-changes --file -` + For example: `ytt --file . | kapp deploy --yes --app local-user-authenticator --diff-changes --file -` ## Configuring After Installing ### Create Users Use `kubectl` to create, edit, and delete user accounts by creating a `Secret` for each user account in the same -namespace where `test-webhook` is deployed. The name of the `Secret` resource is the username. +namespace where `local-user-authenticator` is deployed. The name of the `Secret` resource is the username. Store the user's group membership and `bcrypt` encrypted password as the contents of the `Secret`. For example, to create a user named `ryan` with the password `password123` who belongs to the groups `group1` and `group2`, use: ```bash kubectl create secret generic ryan \ - --namespace test-webhook \ + --namespace local-user-authenticator \ --from-literal=groups=group1,group2 \ --from-literal=passwordHash=$(htpasswd -nbBC 10 x password123 | sed -e "s/^x://") ``` -### Get the `test-webhook` App's Auto-Generated Certificate Authority Bundle +### Get the `local-user-authenticator` App's Auto-Generated Certificate Authority Bundle -Fetch the auto-generated CA bundle for the `test-webhook`'s HTTP TLS endpoint. +Fetch the auto-generated CA bundle for the `local-user-authenticator`'s HTTP TLS endpoint. ```bash -kubectl get secret api-serving-cert --namespace test-webhook \ +kubectl get secret api-serving-cert --namespace local-user-authenticator \ -o jsonpath={.data.caCertificate} \ | base64 -d \ - | tee /tmp/test-webhook-ca + | tee /tmp/local-user-authenticator-ca ``` -### Configuring Pinniped to Use `test-webhook` as an Identity Provider +### Configuring Pinniped to Use `local-user-authenticator` as an Identity Provider -When installing Pinniped on the same cluster, configure `test-webhook` as an Identity Provider for Pinniped -using the webhook URL `https://test-webhook.test-webhook.svc/authenticate` +When installing Pinniped on the same cluster, configure `local-user-authenticator` as an Identity Provider for Pinniped +using the webhook URL `https://local-user-authenticator.local-user-authenticator.svc/authenticate` along with the CA bundle fetched by the above command. ### Optional: Manually Test the Webhook Endpoint @@ -76,14 +76,14 @@ along with the CA bundle fetched by the above command. 1. Copy the CA bundle that was fetched above onto the new pod. ```bash - kubectl cp /tmp/test-webhook-ca curlpod:/tmp/test-webhook-ca + kubectl cp /tmp/local-user-authenticator-ca curlpod:/tmp/local-user-authenticator-ca ``` 1. Run a `curl` command to try to authenticate as the user created above. ```bash - kubectl -it exec curlpod -- curl https://test-webhook.test-webhook.svc/authenticate \ - --cacert /tmp/test-webhook-ca \ + kubectl -it exec curlpod -- curl https://local-user-authenticator.local-user-authenticator.svc/authenticate \ + --cacert /tmp/local-user-authenticator-ca \ -H 'Content-Type: application/json' -H 'Accept: application/json' -d ' { "apiVersion": "authentication.k8s.io/v1beta1", diff --git a/deploy-test-webhook/deployment.yaml b/deploy-local-user-authenticator/deployment.yaml similarity index 57% rename from deploy-test-webhook/deployment.yaml rename to deploy-local-user-authenticator/deployment.yaml index b9a794a2..aed4257b 100644 --- a/deploy-test-webhook/deployment.yaml +++ b/deploy-local-user-authenticator/deployment.yaml @@ -7,36 +7,36 @@ apiVersion: v1 kind: Namespace metadata: - name: test-webhook + name: local-user-authenticator labels: - name: test-webhook + name: local-user-authenticator --- apiVersion: v1 kind: ServiceAccount metadata: - name: test-webhook-service-account - namespace: test-webhook + name: local-user-authenticator-service-account + namespace: local-user-authenticator --- apiVersion: apps/v1 kind: Deployment metadata: - name: test-webhook - namespace: test-webhook + name: local-user-authenticator + namespace: local-user-authenticator labels: - app: test-webhook + app: local-user-authenticator spec: replicas: 1 selector: matchLabels: - app: test-webhook + app: local-user-authenticator template: metadata: labels: - app: test-webhook + app: local-user-authenticator spec: - serviceAccountName: test-webhook-service-account + serviceAccountName: local-user-authenticator-service-account containers: - - name: test-webhook + - name: local-user-authenticator #@ if data.values.image_digest: image: #@ data.values.image_repo + "@" + data.values.image_digest #@ else: @@ -44,19 +44,19 @@ spec: #@ end imagePullPolicy: IfNotPresent command: #! override the default entrypoint - - /usr/local/bin/test-webhook + - /usr/local/bin/local-user-authenticator --- apiVersion: v1 kind: Service metadata: - name: test-webhook - namespace: test-webhook + name: local-user-authenticator + namespace: local-user-authenticator labels: - app: test-webhook + app: local-user-authenticator spec: type: ClusterIP selector: - app: test-webhook + app: local-user-authenticator ports: - protocol: TCP port: 443 diff --git a/deploy-test-webhook/rbac.yaml b/deploy-local-user-authenticator/rbac.yaml similarity index 58% rename from deploy-test-webhook/rbac.yaml rename to deploy-local-user-authenticator/rbac.yaml index a111c5a1..27ad1c2f 100644 --- a/deploy-test-webhook/rbac.yaml +++ b/deploy-local-user-authenticator/rbac.yaml @@ -8,8 +8,8 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: - name: test-webhook-aggregated-api-server-role - namespace: test-webhook + name: local-user-authenticator-aggregated-api-server-role + namespace: local-user-authenticator rules: - apiGroups: [""] resources: [secrets] @@ -18,13 +18,13 @@ rules: kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: - name: test-webhook-aggregated-api-server-role-binding - namespace: test-webhook + name: local-user-authenticator-aggregated-api-server-role-binding + namespace: local-user-authenticator subjects: - kind: ServiceAccount - name: test-webhook-service-account - namespace: test-webhook + name: local-user-authenticator-service-account + namespace: local-user-authenticator roleRef: kind: Role - name: test-webhook-aggregated-api-server-role + name: local-user-authenticator-aggregated-api-server-role apiGroup: rbac.authorization.k8s.io diff --git a/deploy-test-webhook/values.yaml b/deploy-local-user-authenticator/values.yaml similarity index 100% rename from deploy-test-webhook/values.yaml rename to deploy-local-user-authenticator/values.yaml diff --git a/deploy/README.md b/deploy/README.md index ba16679f..96b0c344 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -4,7 +4,7 @@ If you would like to try Pinniped, but you don't have a compatible identity provider, you can use Pinniped's test identity provider. -See [../deploy-test-webhook/README.md](../deploy-test-webhook/README.md) +See [deploy-local-user-authenticator/README.md](../deploy-local-user-authenticator/README.md) for details. ## Tools diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index db885112..d7f2c777 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -212,24 +212,24 @@ else manifest=/tmp/manifest.yaml # - # Deploy test-webhook + # Deploy local-user-authenticator # - pushd deploy-test-webhook >/dev/null + pushd deploy-local-user-authenticator >/dev/null - log_note "Deploying the test-webhook app to the cluster..." + log_note "Deploying the local-user-authenticator app to the cluster..." ytt --file . \ --data-value "image_repo=$registry_repo" \ --data-value "image_tag=$tag" >"$manifest" echo - log_note "Full test-webhook app manifest with Secrets redacted..." + log_note "Full local-user-authenticator app manifest with Secrets redacted..." echo "--------------------------------------------------------------------------------" print_redacted_manifest $manifest echo "--------------------------------------------------------------------------------" echo kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. - kapp deploy --yes --app test-webhook --diff-changes --file "$manifest" + kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" popd >/dev/null @@ -239,7 +239,7 @@ else test_password="test-password" test_groups="test-group-0,test-group-1" kubectl create secret generic "$test_username" \ - --namespace test-webhook \ + --namespace local-user-authenticator \ --from-literal=groups="$test_groups" \ --from-literal=passwordHash="$(htpasswd -nbBC 10 x "$test_password" | sed -e "s/^x://")" \ --dry-run=client \ @@ -248,8 +248,8 @@ else app_name="pinniped" namespace="integration" - webhook_url="https://test-webhook.test-webhook.svc/authenticate" - webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace test-webhook -o 'jsonpath={.data.caCertificate}')" + webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authenticate" + webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')" discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" # @@ -312,5 +312,5 @@ EOF log_note log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." log_note - log_note "To delete the deployments, run 'kapp delete -a test-webhook -y && kapp delete -a pinniped -y'." + log_note "To delete the deployments, run 'kapp delete -a local-user-authenticator -y && kapp delete -a pinniped -y'." fi From e6cb2f8220c19246184f59ecbdc8a060a58707a7 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 10 Sep 2020 17:10:27 -0700 Subject: [PATCH 10/14] Assert on specific expected username and groups in integration tests Signed-off-by: Ryan Richard --- hack/prepare-for-integration-tests.sh | 57 +++++----------------- test/integration/client_test.go | 2 +- test/integration/credentialrequest_test.go | 18 ++++--- 3 files changed, 24 insertions(+), 53 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index d7f2c777..35ef1c20 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -12,34 +12,6 @@ set -euo pipefail -function print_or_redact_doc() { - doc_kind=$(echo "$1" | awk '/^kind: / {print $2}') - if [[ -z "$doc_kind" ]]; then - echo "warning: " - elif [[ $doc_kind == "Secret" || $doc_kind == "secret" ]]; then - echo - echo "---" - echo "" - else - printf "%s\n" "$1" - fi -} - -function print_redacted_manifest() { - doc="" - while IFS="" read -r line || [ -n "$line" ]; do - if [[ $line == "---" ]]; then - if [[ -n "$doc" ]]; then - print_or_redact_doc "$doc" - fi - doc="" - fi - doc=$(printf "%s\n%s" "$doc" "$line") - done <"$1" - - print_or_redact_doc "$doc" -} - function log_note() { GREEN='\033[0;32m' NC='\033[0m' @@ -221,23 +193,21 @@ else --data-value "image_repo=$registry_repo" \ --data-value "image_tag=$tag" >"$manifest" - echo - log_note "Full local-user-authenticator app manifest with Secrets redacted..." - echo "--------------------------------------------------------------------------------" - print_redacted_manifest $manifest - echo "--------------------------------------------------------------------------------" - echo - kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" popd >/dev/null - log_note "Creating test user 'test-username'..." test_username="test-username" - # TODO AUTO-GENERATE PASSWORD - test_password="test-password" test_groups="test-group-0,test-group-1" + set +o pipefail + test_password="$(cat /dev/urandom | env LC_CTYPE=C tr -dc 'a-z0-9' | fold -w 32 | head -n 1)" + set -o pipefail + if [[ ${#test_password} -ne 32 ]]; then + log_error "Could not create random test user password" + exit 1 + fi + echo "Creating test user '$test_username'..." kubectl create secret generic "$test_username" \ --namespace local-user-authenticator \ --from-literal=groups="$test_groups" \ @@ -267,13 +237,6 @@ else --data-value "webhook_ca_bundle=$webhook_ca_bundle" \ --data-value "discovery_url=$discovery_url" >"$manifest" - echo - log_note "Full Pinniped app manifest with Secrets redacted..." - echo "--------------------------------------------------------------------------------" - print_redacted_manifest $manifest - echo "--------------------------------------------------------------------------------" - echo - kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" @@ -286,7 +249,9 @@ else # The following env vars should be set before running 'cd test && go test ./...' export PINNIPED_NAMESPACE=${namespace} export PINNIPED_APP_NAME=${app_name} -export PINNIPED_CREDENTIAL_REQUEST_TOKEN=${test_username}:${test_password} +export PINNIPED_TEST_USER_USERNAME=${test_username} +export PINNIPED_TEST_USER_GROUPS=${test_groups} +export PINNIPED_TEST_USER_TOKEN=${test_username}:${test_password} read -r -d '' PINNIPED_CLUSTER_CAPABILITY_YAML << PINNIPED_CLUSTER_CAPABILITY_YAML_EOF || true ${pinniped_cluster_capability_file_content} diff --git a/test/integration/client_test.go b/test/integration/client_test.go index da8e202a..ad480d1a 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -57,7 +57,7 @@ var maskKey = func(s string) string { return strings.ReplaceAll(s, "TESTING KEY" func TestClient(t *testing.T) { library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - token := library.GetEnv(t, "PINNIPED_CREDENTIAL_REQUEST_TOKEN") + token := library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index 5dce437b..e3e3ff50 100644 --- a/test/integration/credentialrequest_test.go +++ b/test/integration/credentialrequest_test.go @@ -10,6 +10,7 @@ import ( "crypto/x509" "encoding/pem" "net/http" + "strings" "testing" "time" @@ -28,6 +29,10 @@ import ( func TestSuccessfulCredentialRequest(t *testing.T) { library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) + testUsername := library.GetEnv(t, "PINNIPED_TEST_USER_USERNAME") + expectedTestUserGroups := strings.Split( + strings.ReplaceAll(library.GetEnv(t, "PINNIPED_TEST_USER_GROUPS"), " ", ""), ",", + ) response, err := makeRequest(t, validCredentialRequestSpecWithRealToken(t)) require.NoError(t, err) @@ -39,6 +44,8 @@ func TestSuccessfulCredentialRequest(t *testing.T) { require.NotNil(t, response.Status.Credential) require.Empty(t, response.Status.Credential.Token) require.NotEmpty(t, response.Status.Credential.ClientCertificateData) + require.Equal(t, testUsername, getCommonName(t, response.Status.Credential.ClientCertificateData)) + require.ElementsMatch(t, expectedTestUserGroups, getOrganizations(t, response.Status.Credential.ClientCertificateData)) require.NotEmpty(t, response.Status.Credential.ClientKeyData) require.NotNil(t, response.Status.Credential.ExpirationTimestamp) require.InDelta(t, time.Until(response.Status.Credential.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute)) @@ -65,7 +72,7 @@ func TestSuccessfulCredentialRequest(t *testing.T) { Subjects: []rbacv1.Subject{{ Kind: rbacv1.UserKind, APIGroup: rbacv1.GroupName, - Name: getCommonName(t, response.Status.Credential.ClientCertificateData), + Name: testUsername, }}, RoleRef: rbacv1.RoleRef{ Kind: "ClusterRole", @@ -85,7 +92,7 @@ func TestSuccessfulCredentialRequest(t *testing.T) { require.NotEmpty(t, listNamespaceResponse.Items) }) - for _, group := range getOrganizationalUnits(t, response.Status.Credential.ClientCertificateData) { + for _, group := range expectedTestUserGroups { t.Run("access as group "+group, func(t *testing.T) { addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ TypeMeta: metav1.TypeMeta{}, @@ -185,8 +192,7 @@ func makeRequest(t *testing.T, spec v1alpha1.CredentialRequestSpec) (*v1alpha1.C } func validCredentialRequestSpecWithRealToken(t *testing.T) v1alpha1.CredentialRequestSpec { - token := library.GetEnv(t, "PINNIPED_CREDENTIAL_REQUEST_TOKEN") - + token := library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN") return v1alpha1.CredentialRequestSpec{ Type: v1alpha1.TokenCredentialType, Token: &v1alpha1.CredentialRequestTokenCredential{Value: token}, @@ -227,12 +233,12 @@ func getCommonName(t *testing.T, certPEM string) string { return cert.Subject.CommonName } -func getOrganizationalUnits(t *testing.T, certPEM string) []string { +func getOrganizations(t *testing.T, certPEM string) []string { t.Helper() pemBlock, _ := pem.Decode([]byte(certPEM)) cert, err := x509.ParseCertificate(pemBlock.Bytes) require.NoError(t, err) - return cert.Subject.OrganizationalUnit + return cert.Subject.Organization } From 4fe609a043de9058f59cfc2e2f880efbe30a846d Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 10 Sep 2020 17:36:22 -0700 Subject: [PATCH 11/14] Remove mentions of uninstall tests and other repos from prepare-for-integration-tests.sh --- hack/prepare-for-integration-tests.sh | 198 ++++++++++++-------------- 1 file changed, 90 insertions(+), 108 deletions(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 35ef1c20..d8828b58 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -2,16 +2,13 @@ # This script can be used to prepare a kind cluster and deploy the app. # You can call this script again to redeploy the app. -# It will also output instructions on how to run the integration or uninstall tests. - -# TODO: get rid of references to ci repo -# TODO: fix uninstall test setup -# TODO: add flag to let user provide registry/tag for their image -# \- (this can be used by kind integration tests from kind-load-and-docker-run-any-script.sh) -# TODO: add flag to setup current-context for integration tests +# It will also output instructions on how to run the integration. set -euo pipefail +# +# Helper functions +# function log_note() { GREEN='\033[0;32m' NC='\033[0m' @@ -42,9 +39,11 @@ function log_error() { fi } +# +# Handle argument parsing and help message +# help=no skip_build=no -prepare_for_uninstall_test=no PARAMS="" while (("$#")); do @@ -57,10 +56,6 @@ while (("$#")); do skip_build=yes shift ;; - -u | --prepare-uninstall) - prepare_for_uninstall_test=yes - shift - ;; -*) log_error "Unsupported flag $1" >&2 exit 1 @@ -76,19 +71,21 @@ eval set -- "$PARAMS" if [[ "$help" == "yes" ]]; then me="$(basename "${BASH_SOURCE[0]}")" echo "Usage:" - echo " $me [flags] [path/to/pinniped] [path/to/pinniped-ci]" + echo " $me [flags] [path/to/pinniped]" echo echo " path/to/pinniped default: \$PWD ($PWD)" echo echo "Flags:" echo " -h, --help: print this usage" echo " -s, --skip-build: reuse the most recently built image of the app instead of building" - echo " -u, --prepare-uninstall: delete the kind cluster and prepare to run the install+uninstall test" exit 1 fi pinniped_path="${1-$PWD}" +# +# Check for dependencies +# if ! command -v kind >/dev/null; then log_error "Please install kind. e.g. 'brew install kind' for MacOS" exit 1 @@ -116,11 +113,9 @@ if [[ ! -f Dockerfile || ! -d deploy ]]; then exit 1 fi -if [[ "$prepare_for_uninstall_test" == "yes" ]]; then - log_note "Deleting running kind clusters to prepare a clean slate for the install+uninstall test..." - kind delete cluster -fi - +# +# Setup kind and build the app +# log_note "Checking for running kind clusters..." if ! kind get clusters | grep -q -e '^kind$'; then log_note "Creating a kind cluster..." @@ -163,89 +158,74 @@ fi log_note "Loading the app's container image into the kind cluster..." kind load docker-image "$registry_repo_tag" -if [[ "$prepare_for_uninstall_test" == "yes" ]]; then - cat </tmp/uninstall-test-env -# The following env vars should be set before running $pinniped_ci_path/pipelines/shared-tasks/run-uninstall-test/run-uninstall-test.sh -export IMAGE_REPO="$registry_repo" -export IMAGE_TAG="$tag" -export PINNIPED_DISCOVERY_URL="$discovery_url" -EOF +manifest=/tmp/manifest.yaml - log_note "Done!" - log_note - log_note "Ready to run the uninstall test." - log_note " cd $pinniped_path" - log_note ' source /tmp/uninstall-test-env' - log_note " $pinniped_ci_path/pipelines/shared-tasks/run-uninstall-test/run-uninstall-test.sh" - log_note - log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." +# +# Deploy local-user-authenticator +# +pushd deploy-local-user-authenticator >/dev/null -else - manifest=/tmp/manifest.yaml +log_note "Deploying the local-user-authenticator app to the cluster..." +ytt --file . \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" >"$manifest" - # - # Deploy local-user-authenticator - # - pushd deploy-local-user-authenticator >/dev/null +kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. +kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" - log_note "Deploying the local-user-authenticator app to the cluster..." - ytt --file . \ - --data-value "image_repo=$registry_repo" \ - --data-value "image_tag=$tag" >"$manifest" +popd >/dev/null - kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. - kapp deploy --yes --app local-user-authenticator --diff-changes --file "$manifest" +test_username="test-username" +test_groups="test-group-0,test-group-1" +set +o pipefail +test_password="$(cat /dev/urandom | env LC_CTYPE=C tr -dc 'a-z0-9' | fold -w 32 | head -n 1)" +set -o pipefail +if [[ ${#test_password} -ne 32 ]]; then + log_error "Could not create random test user password" + exit 1 +fi +echo "Creating test user '$test_username'..." +kubectl create secret generic "$test_username" \ + --namespace local-user-authenticator \ + --from-literal=groups="$test_groups" \ + --from-literal=passwordHash="$(htpasswd -nbBC 10 x "$test_password" | sed -e "s/^x://")" \ + --dry-run=client \ + --output yaml | + kubectl apply -f - - popd >/dev/null +app_name="pinniped" +namespace="integration" +webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authenticate" +webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')" +discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" - test_username="test-username" - test_groups="test-group-0,test-group-1" - set +o pipefail - test_password="$(cat /dev/urandom | env LC_CTYPE=C tr -dc 'a-z0-9' | fold -w 32 | head -n 1)" - set -o pipefail - if [[ ${#test_password} -ne 32 ]]; then - log_error "Could not create random test user password" - exit 1 - fi - echo "Creating test user '$test_username'..." - kubectl create secret generic "$test_username" \ - --namespace local-user-authenticator \ - --from-literal=groups="$test_groups" \ - --from-literal=passwordHash="$(htpasswd -nbBC 10 x "$test_password" | sed -e "s/^x://")" \ - --dry-run=client \ - --output yaml \ - | kubectl apply -f - +# +# Deploy Pinniped +# +pushd deploy >/dev/null - app_name="pinniped" - namespace="integration" - webhook_url="https://local-user-authenticator.local-user-authenticator.svc/authenticate" - webhook_ca_bundle="$(kubectl get secret api-serving-cert --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')" - discovery_url="$(TERM=dumb kubectl cluster-info | awk '/Kubernetes master/ {print $NF}')" +log_note "Deploying the Pinniped app to the cluster..." +ytt --file . \ + --data-value "app_name=$app_name" \ + --data-value "namespace=$namespace" \ + --data-value "image_repo=$registry_repo" \ + --data-value "image_tag=$tag" \ + --data-value "webhook_url=$webhook_url" \ + --data-value "webhook_ca_bundle=$webhook_ca_bundle" \ + --data-value "discovery_url=$discovery_url" >"$manifest" - # - # Deploy Pinniped - # - pushd deploy >/dev/null +kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. +kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" - log_note "Deploying the Pinniped app to the cluster..." - ytt --file . \ - --data-value "app_name=$app_name" \ - --data-value "namespace=$namespace" \ - --data-value "image_repo=$registry_repo" \ - --data-value "image_tag=$tag" \ - --data-value "webhook_url=$webhook_url" \ - --data-value "webhook_ca_bundle=$webhook_ca_bundle" \ - --data-value "discovery_url=$discovery_url" >"$manifest" +popd >/dev/null - kubectl apply --dry-run=client -f "$manifest" # Validate manifest schema. - kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" +# +# Create the environment file +# +kind_capabilities_file="$pinniped_path/test/cluster_capabilities/kind.yaml" +pinniped_cluster_capability_file_content=$(cat "$kind_capabilities_file") - popd >/dev/null - - kind_capabilities_file="$pinniped_path/test/cluster_capabilities/kind.yaml" - pinniped_cluster_capability_file_content=$(cat "$kind_capabilities_file") - - cat </tmp/integration-test-env +cat </tmp/integration-test-env # The following env vars should be set before running 'cd test && go test ./...' export PINNIPED_NAMESPACE=${namespace} export PINNIPED_APP_NAME=${app_name} @@ -260,22 +240,24 @@ PINNIPED_CLUSTER_CAPABILITY_YAML_EOF export PINNIPED_CLUSTER_CAPABILITY_YAML EOF - goland_vars=$(grep -v '^#' /tmp/integration-test-env | grep -E '^export .+=' | sed 's/export //g' | tr '\n' ';') +# +# Print instructions for next steps +# +goland_vars=$(grep -v '^#' /tmp/integration-test-env | grep -E '^export .+=' | sed 's/export //g' | tr '\n' ';') - log_note "Done!" - log_note - log_note "Ready to run integration tests. For example, you could run all tests using the following commands..." - log_note " cd $pinniped_path" - log_note ' source /tmp/integration-test-env' - log_note ' (cd test && go test -count 1 ./...)' - log_note - log_note '"Environment" setting for GoLand run configurations:' - log_note " ${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" - log_note - log_note - log_note "You can run this script again to deploy local production code changes while you are working." - log_note - log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." - log_note - log_note "To delete the deployments, run 'kapp delete -a local-user-authenticator -y && kapp delete -a pinniped -y'." -fi +log_note "Done!" +log_note +log_note "Ready to run integration tests. For example, you could run all tests using the following commands..." +log_note " cd $pinniped_path" +log_note ' source /tmp/integration-test-env' +log_note ' (cd test && go test -count 1 ./...)' +log_note +log_note '"Environment" setting for GoLand run configurations:' +log_note " ${goland_vars}PINNIPED_CLUSTER_CAPABILITY_FILE=${kind_capabilities_file}" +log_note +log_note +log_note "You can run this script again to deploy local production code changes while you are working." +log_note +log_note "When you're finished, use 'kind delete cluster' to tear down the cluster." +log_note +log_note "To delete the deployments, run 'kapp delete -a local-user-authenticator -y && kapp delete -a pinniped -y'." From 6deaa0fb1a769ac1b8cd4825272cb145faa4ea1b Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 10 Sep 2020 18:34:18 -0700 Subject: [PATCH 12/14] Fix lint errors --- cmd/local-user-authenticator/main.go | 92 ++++++++++++---------- cmd/local-user-authenticator/main_test.go | 6 +- test/integration/credentialrequest_test.go | 1 + 3 files changed, 54 insertions(+), 45 deletions(-) diff --git a/cmd/local-user-authenticator/main.go b/cmd/local-user-authenticator/main.go index b5d1400e..4d66ea4e 100644 --- a/cmd/local-user-authenticator/main.go +++ b/cmd/local-user-authenticator/main.go @@ -35,6 +35,7 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/klog/v2" + "github.com/suzerain-io/pinniped/internal/constable" "github.com/suzerain-io/pinniped/internal/controller/apicerts" "github.com/suzerain-io/pinniped/internal/controllerlib" "github.com/suzerain-io/pinniped/internal/provider" @@ -46,14 +47,13 @@ const ( // This string must match the name of the Service declared in the deployment yaml. serviceName = "local-user-authenticator" - // TODO there must be a better way to get this specific json result string without needing to hardcode it - unauthenticatedResponse = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":false}}` - - // TODO there must be a better way to get this specific json result string without needing to hardcode it + unauthenticatedResponse = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":false}}` authenticatedResponseTemplate = `{"apiVersion":"authentication.k8s.io/v1beta1","kind":"TokenReview","status":{"authenticated":true,"user":{"username":"%s","uid":"%s","groups":%s}}}` singletonWorker = 1 defaultResyncInterval = 3 * time.Minute + + invalidRequest = constable.Error("invalid request") ) type webhook struct { @@ -111,47 +111,11 @@ func (w *webhook) start(ctx context.Context, l net.Listener) error { func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { defer req.Body.Close() - if req.URL.Path != "/authenticate" { - klog.InfoS("received request path other than /authenticate", "path", req.URL.Path) - rsp.WriteHeader(http.StatusNotFound) + username, password, err := getUsernameAndPasswordFromRequest(rsp, req) + if err != nil { return } - if req.Method != http.MethodPost { - klog.InfoS("received request method other than post", "method", req.Method) - rsp.WriteHeader(http.StatusMethodNotAllowed) - return - } - - if !headerContains(req, "Content-Type", "application/json") { - klog.InfoS("content type is not application/json", "Content-Type", req.Header.Values("Content-Type")) - rsp.WriteHeader(http.StatusUnsupportedMediaType) - return - } - if !headerContains(req, "Accept", "application/json") && - !headerContains(req, "Accept", "application/*") && - !headerContains(req, "Accept", "*/*") { - klog.InfoS("client does not accept application/json", "Accept", req.Header.Values("Accept")) - rsp.WriteHeader(http.StatusUnsupportedMediaType) - return - } - - var body authenticationv1.TokenReview - if err := json.NewDecoder(req.Body).Decode(&body); err != nil { - klog.InfoS("failed to decode body", "err", err) - rsp.WriteHeader(http.StatusBadRequest) - return - } - - tokenSegments := strings.SplitN(body.Spec.Token, ":", 2) - if len(tokenSegments) != 2 { - klog.InfoS("bad token format in request") - rsp.WriteHeader(http.StatusBadRequest) - return - } - username := tokenSegments[0] - password := tokenSegments[1] - secret, err := w.secretInformer.Lister().Secrets(namespace).Get(username) notFound := k8serrors.IsNotFound(err) if err != nil && !notFound { @@ -193,6 +157,50 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups) } +func getUsernameAndPasswordFromRequest(rsp http.ResponseWriter, req *http.Request) (string, string, error) { + if req.URL.Path != "/authenticate" { + klog.InfoS("received request path other than /authenticate", "path", req.URL.Path) + rsp.WriteHeader(http.StatusNotFound) + return "", "", invalidRequest + } + + if req.Method != http.MethodPost { + klog.InfoS("received request method other than post", "method", req.Method) + rsp.WriteHeader(http.StatusMethodNotAllowed) + return "", "", invalidRequest + } + + if !headerContains(req, "Content-Type", "application/json") { + klog.InfoS("content type is not application/json", "Content-Type", req.Header.Values("Content-Type")) + rsp.WriteHeader(http.StatusUnsupportedMediaType) + return "", "", invalidRequest + } + + if !headerContains(req, "Accept", "application/json") && + !headerContains(req, "Accept", "application/*") && + !headerContains(req, "Accept", "*/*") { + klog.InfoS("client does not accept application/json", "Accept", req.Header.Values("Accept")) + rsp.WriteHeader(http.StatusUnsupportedMediaType) + return "", "", invalidRequest + } + + var body authenticationv1.TokenReview + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + klog.InfoS("failed to decode body", "err", err) + rsp.WriteHeader(http.StatusBadRequest) + return "", "", invalidRequest + } + + tokenSegments := strings.SplitN(body.Spec.Token, ":", 2) + if len(tokenSegments) != 2 { + klog.InfoS("bad token format in request") + rsp.WriteHeader(http.StatusBadRequest) + return "", "", invalidRequest + } + + return tokenSegments[0], tokenSegments[1], nil +} + func headerContains(req *http.Request, headerName, s string) bool { headerValues := req.Header.Values(headerName) for i := range headerValues { diff --git a/cmd/local-user-authenticator/main_test.go b/cmd/local-user-authenticator/main_test.go index 4ac44fa6..0bc49255 100644 --- a/cmd/local-user-authenticator/main_test.go +++ b/cmd/local-user-authenticator/main_test.go @@ -451,9 +451,9 @@ func unauthenticatedResponseJSON() *string { } func authenticatedResponseJSON(user, uid string, groups []string) *string { - var quotedGroups []string - for _, group := range groups { - quotedGroups = append(quotedGroups, `"`+group+`"`) + quotedGroups := make([]string, len(groups)) + for i, group := range groups { + quotedGroups[i] = `"` + group + `"` } // Very specific expected result. Avoid using json package so we know exactly what we're asserting. diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index e3e3ff50..cb9b0418 100644 --- a/test/integration/credentialrequest_test.go +++ b/test/integration/credentialrequest_test.go @@ -93,6 +93,7 @@ func TestSuccessfulCredentialRequest(t *testing.T) { }) for _, group := range expectedTestUserGroups { + group := group t.Run("access as group "+group, func(t *testing.T) { addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ TypeMeta: metav1.TypeMeta{}, From 22bf24b775e5ee460fcf8715ca9a52901f68d946 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 10 Sep 2020 18:50:34 -0700 Subject: [PATCH 13/14] Fix a unit test failure that only happens on golang 1.15 - Use the SAN field when creating a test cert or else the corresponding unit tests will fail when run with golang 1.15 --- cmd/local-user-authenticator/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/local-user-authenticator/main_test.go b/cmd/local-user-authenticator/main_test.go index 0bc49255..837b3c2a 100644 --- a/cmd/local-user-authenticator/main_test.go +++ b/cmd/local-user-authenticator/main_test.go @@ -395,7 +395,7 @@ func newCertProvider(t *testing.T) (provider.DynamicTLSServingCertProvider, []by serverName := "local-user-authenticator" cert, err := ca.Issue( pkix.Name{CommonName: serverName}, - []string{}, + []string{serverName}, time.Hour*24, ) require.NoError(t, err) From 4fa7e1bd7687c827a5ed844922e142e6b57d90fb Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Fri, 11 Sep 2020 10:09:13 -0400 Subject: [PATCH 14/14] hack/prepare-for-integration-tests.sh: use log helper Signed-off-by: Andrew Keesler --- hack/prepare-for-integration-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index d8828b58..df63773d 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -184,7 +184,7 @@ if [[ ${#test_password} -ne 32 ]]; then log_error "Could not create random test user password" exit 1 fi -echo "Creating test user '$test_username'..." +log_note "Creating test user '$test_username'..." kubectl create secret generic "$test_username" \ --namespace local-user-authenticator \ --from-literal=groups="$test_groups" \