8b7c30cfbd
- TLS certificates can be configured on the OIDCProviderConfig using the `secretName` field. - When listening for incoming TLS connections, choose the TLS cert based on the SNI hostname of the incoming request. - Because SNI hostname information on incoming requests does not include the port number of the request, we add a validation that OIDCProviderConfigs where the issuer hostnames (not including port number) are the same must use the same `secretName`. - Note that this approach does not yet support requests made to an IP address instead of a hostname. Also note that `localhost` is considered a hostname by SNI. - Add port 443 as a container port to the pod spec. - A new controller watches for TLS secrets and caches them in memory. That same in-memory cache is used while servicing incoming connections on the TLS port. - Make it easy to configure both port 443 and/or port 80 for various Service types using our ytt templates for the supervisor. - When deploying to kind, add another nodeport and forward it to the host on another port to expose our new HTTPS supervisor port to the host.
305 lines
12 KiB
Go
305 lines
12 KiB
Go
// Copyright 2020 the Pinniped contributors. All Rights Reserved.
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
|
|
package supervisorconfig
|
|
|
|
import (
|
|
"context"
|
|
"crypto/tls"
|
|
"io/ioutil"
|
|
"net/url"
|
|
"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"
|
|
kubeinformers "k8s.io/client-go/informers"
|
|
kubernetesfake "k8s.io/client-go/kubernetes/fake"
|
|
|
|
"go.pinniped.dev/generated/1.19/apis/config/v1alpha1"
|
|
pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake"
|
|
pinnipedinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions"
|
|
"go.pinniped.dev/internal/controllerlib"
|
|
"go.pinniped.dev/internal/testutil"
|
|
)
|
|
|
|
func TestTLSCertObserverControllerInformerFilters(t *testing.T) {
|
|
spec.Run(t, "informer filters", func(t *testing.T, when spec.G, it spec.S) {
|
|
var (
|
|
r *require.Assertions
|
|
observableWithInformerOption *testutil.ObservableWithInformerOption
|
|
secretsInformerFilter controllerlib.Filter
|
|
oidcProviderConfigInformerFilter controllerlib.Filter
|
|
)
|
|
|
|
it.Before(func() {
|
|
r = require.New(t)
|
|
observableWithInformerOption = testutil.NewObservableWithInformerOption()
|
|
secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()
|
|
oidcProviderConfigInformer := pinnipedinformers.NewSharedInformerFactory(nil, 0).Config().V1alpha1().OIDCProviderConfigs()
|
|
_ = NewTLSCertObserverController(
|
|
nil,
|
|
secretsInformer,
|
|
oidcProviderConfigInformer,
|
|
observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters
|
|
)
|
|
secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer)
|
|
oidcProviderConfigInformerFilter = observableWithInformerOption.GetFilterForInformer(oidcProviderConfigInformer)
|
|
})
|
|
|
|
when("watching Secret objects", func() {
|
|
var (
|
|
subject controllerlib.Filter
|
|
secret, otherSecret *corev1.Secret
|
|
)
|
|
|
|
it.Before(func() {
|
|
subject = secretsInformerFilter
|
|
secret = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "any-name", Namespace: "any-namespace"}}
|
|
otherSecret = &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "any-other-name", Namespace: "any-other-namespace"}}
|
|
})
|
|
|
|
when("any Secret changes", func() {
|
|
it("returns true to trigger the sync method", func() {
|
|
r.True(subject.Add(secret))
|
|
r.True(subject.Update(secret, otherSecret))
|
|
r.True(subject.Update(otherSecret, secret))
|
|
r.True(subject.Delete(secret))
|
|
})
|
|
})
|
|
})
|
|
|
|
when("watching OIDCProviderConfig objects", func() {
|
|
var (
|
|
subject controllerlib.Filter
|
|
provider, otherProvider *v1alpha1.OIDCProviderConfig
|
|
)
|
|
|
|
it.Before(func() {
|
|
subject = oidcProviderConfigInformerFilter
|
|
provider = &v1alpha1.OIDCProviderConfig{ObjectMeta: metav1.ObjectMeta{Name: "any-name", Namespace: "any-namespace"}}
|
|
otherProvider = &v1alpha1.OIDCProviderConfig{ObjectMeta: metav1.ObjectMeta{Name: "any-other-name", Namespace: "any-other-namespace"}}
|
|
})
|
|
|
|
when("any OIDCProviderConfig changes", func() {
|
|
it("returns true to trigger the sync method", func() {
|
|
r.True(subject.Add(provider))
|
|
r.True(subject.Update(provider, otherProvider))
|
|
r.True(subject.Update(otherProvider, provider))
|
|
r.True(subject.Delete(provider))
|
|
})
|
|
})
|
|
})
|
|
}, spec.Parallel(), spec.Report(report.Terminal{}))
|
|
}
|
|
|
|
type fakeIssuerHostToTLSCertMapSetter struct {
|
|
setIssuerHostToTLSCertMapWasCalled bool
|
|
issuerHostToTLSCertMapReceived map[string]*tls.Certificate
|
|
}
|
|
|
|
func (f *fakeIssuerHostToTLSCertMapSetter) SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap map[string]*tls.Certificate) {
|
|
f.setIssuerHostToTLSCertMapWasCalled = true
|
|
f.issuerHostToTLSCertMapReceived = issuerHostToTLSCertMap
|
|
}
|
|
|
|
func TestTLSCertObserverControllerSync(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
|
|
subject controllerlib.Controller
|
|
pinnipedInformerClient *pinnipedfake.Clientset
|
|
kubeInformerClient *kubernetesfake.Clientset
|
|
pinnipedInformers pinnipedinformers.SharedInformerFactory
|
|
kubeInformers kubeinformers.SharedInformerFactory
|
|
timeoutContext context.Context
|
|
timeoutContextCancel context.CancelFunc
|
|
syncContext *controllerlib.Context
|
|
issuerHostToTLSCertSetter *fakeIssuerHostToTLSCertMapSetter
|
|
)
|
|
|
|
// 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 = NewTLSCertObserverController(
|
|
issuerHostToTLSCertSetter,
|
|
kubeInformers.Core().V1().Secrets(),
|
|
pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(),
|
|
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: "any-name",
|
|
},
|
|
}
|
|
|
|
// Must start informers before calling TestRunSynchronously()
|
|
kubeInformers.Start(timeoutContext.Done())
|
|
pinnipedInformers.Start(timeoutContext.Done())
|
|
controllerlib.TestRunSynchronously(t, subject)
|
|
}
|
|
|
|
var readTestFile = func(path string) []byte {
|
|
data, err := ioutil.ReadFile(path)
|
|
r.NoError(err)
|
|
return data
|
|
}
|
|
|
|
it.Before(func() {
|
|
r = require.New(t)
|
|
|
|
timeoutContext, timeoutContextCancel = context.WithTimeout(context.Background(), time.Second*3)
|
|
|
|
kubeInformerClient = kubernetesfake.NewSimpleClientset()
|
|
kubeInformers = kubeinformers.NewSharedInformerFactory(kubeInformerClient, 0)
|
|
pinnipedInformerClient = pinnipedfake.NewSimpleClientset()
|
|
pinnipedInformers = pinnipedinformers.NewSharedInformerFactory(pinnipedInformerClient, 0)
|
|
issuerHostToTLSCertSetter = &fakeIssuerHostToTLSCertMapSetter{}
|
|
|
|
unrelatedSecret := &corev1.Secret{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "some other unrelated secret",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
}
|
|
r.NoError(kubeInformerClient.Tracker().Add(unrelatedSecret))
|
|
})
|
|
|
|
it.After(func() {
|
|
timeoutContextCancel()
|
|
})
|
|
|
|
when("there are no OIDCProviderConfigs and no TLS Secrets yet", func() {
|
|
it("sets the issuerHostToTLSCertSetter's map to be empty", func() {
|
|
startInformersAndController()
|
|
err := controllerlib.TestSync(t, subject, *syncContext)
|
|
r.NoError(err)
|
|
|
|
r.True(issuerHostToTLSCertSetter.setIssuerHostToTLSCertMapWasCalled)
|
|
r.Empty(issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived)
|
|
})
|
|
})
|
|
|
|
when("there are OIDCProviderConfigs where some have corresponding TLS Secrets and some don't", func() {
|
|
var (
|
|
expectedCertificate1, expectedCertificate2 tls.Certificate
|
|
)
|
|
|
|
it.Before(func() {
|
|
var err error
|
|
oidcProviderConfigWithoutSecret1 := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "no-secret-oidcproviderconfig1",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://no-secret-issuer1.com"}, // no SecretName field
|
|
}
|
|
oidcProviderConfigWithoutSecret2 := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "no-secret-oidcproviderconfig2",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://no-secret-issuer2.com", SecretName: ""},
|
|
}
|
|
oidcProviderConfigWithBadSecret := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "bad-secret-oidcproviderconfig",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://bad-secret-issuer.com", SecretName: "bad-tls-secret-name"},
|
|
}
|
|
// Also add one with a URL that cannot be parsed to make sure that the controller is not confused by invalid URLs.
|
|
invalidIssuerURL := ":/host//path"
|
|
_, err = url.Parse(invalidIssuerURL) //nolint:staticcheck // Yes, this URL is intentionally invalid.
|
|
r.Error(err)
|
|
oidcProviderConfigWithBadIssuer := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "bad-issuer-oidcproviderconfig",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: invalidIssuerURL},
|
|
}
|
|
oidcProviderConfigWithGoodSecret1 := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "good-secret-oidcproviderconfig1",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
// Issuer hostname should be treated in a case-insensitive way and SNI ignores port numbers. Test without a port number.
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://www.iSSuer-wiTh-goOd-secRet1.cOm/path", SecretName: "good-tls-secret-name1"},
|
|
}
|
|
oidcProviderConfigWithGoodSecret2 := &v1alpha1.OIDCProviderConfig{
|
|
ObjectMeta: metav1.ObjectMeta{
|
|
Name: "good-secret-oidcproviderconfig2",
|
|
Namespace: installedInNamespace,
|
|
},
|
|
// Issuer hostname should be treated in a case-insensitive way and SNI ignores port numbers. Test with a port number.
|
|
Spec: v1alpha1.OIDCProviderConfigSpec{Issuer: "https://www.issUEr-WIth-gOOd-seCret2.com:1234/path", SecretName: "good-tls-secret-name2"},
|
|
}
|
|
testCrt1 := readTestFile("testdata/test.crt")
|
|
r.NotEmpty(testCrt1)
|
|
testCrt2 := readTestFile("testdata/test2.crt")
|
|
r.NotEmpty(testCrt2)
|
|
testKey1 := readTestFile("testdata/test.key")
|
|
r.NotEmpty(testKey1)
|
|
testKey2 := readTestFile("testdata/test2.key")
|
|
r.NotEmpty(testKey2)
|
|
expectedCertificate1, err = tls.X509KeyPair(testCrt1, testKey1)
|
|
r.NoError(err)
|
|
expectedCertificate2, err = tls.X509KeyPair(testCrt2, testKey2)
|
|
r.NoError(err)
|
|
goodTLSSecret1 := &corev1.Secret{
|
|
ObjectMeta: metav1.ObjectMeta{Name: "good-tls-secret-name1", Namespace: installedInNamespace},
|
|
Data: map[string][]byte{"tls.crt": testCrt1, "tls.key": testKey1},
|
|
}
|
|
goodTLSSecret2 := &corev1.Secret{
|
|
ObjectMeta: metav1.ObjectMeta{Name: "good-tls-secret-name2", Namespace: installedInNamespace},
|
|
Data: map[string][]byte{"tls.crt": testCrt2, "tls.key": testKey2},
|
|
}
|
|
badTLSSecret := &corev1.Secret{
|
|
ObjectMeta: metav1.ObjectMeta{Name: "bad-tls-secret-name", Namespace: installedInNamespace},
|
|
Data: map[string][]byte{"junk": nil},
|
|
}
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithoutSecret1))
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithoutSecret2))
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithBadSecret))
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithBadIssuer))
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithGoodSecret1))
|
|
r.NoError(pinnipedInformerClient.Tracker().Add(oidcProviderConfigWithGoodSecret2))
|
|
r.NoError(kubeInformerClient.Tracker().Add(goodTLSSecret1))
|
|
r.NoError(kubeInformerClient.Tracker().Add(goodTLSSecret2))
|
|
r.NoError(kubeInformerClient.Tracker().Add(badTLSSecret))
|
|
})
|
|
|
|
it("updates the issuerHostToTLSCertSetter's map to include only the issuers that had valid certs", func() {
|
|
startInformersAndController()
|
|
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
|
|
|
|
r.True(issuerHostToTLSCertSetter.setIssuerHostToTLSCertMapWasCalled)
|
|
r.Len(issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived, 2)
|
|
|
|
// They keys in the map should be lower case and should not include the port numbers, because
|
|
// TLS SNI says that SNI hostnames must be DNS names (not ports) and must be case insensitive.
|
|
// See https://tools.ietf.org/html/rfc3546#section-3.1
|
|
actualCertificate1 := issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret1.com"]
|
|
r.NotNil(actualCertificate1)
|
|
// The actual cert should match the one from the test fixture that was put into the secret.
|
|
r.Equal(expectedCertificate1, *actualCertificate1)
|
|
actualCertificate2 := issuerHostToTLSCertSetter.issuerHostToTLSCertMapReceived["www.issuer-with-good-secret2.com"]
|
|
r.NotNil(actualCertificate2)
|
|
r.Equal(expectedCertificate2, *actualCertificate2)
|
|
})
|
|
})
|
|
}, spec.Parallel(), spec.Report(report.Terminal{}))
|
|
}
|