From 29e0ce566263dae09385bb1df692fc3a680f196f Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Wed, 28 Oct 2020 11:56:50 -0700 Subject: [PATCH] Configure name of the supervisor default TLS cert secret via ConfigMap Signed-off-by: Andrew Keesler --- cmd/pinniped-supervisor/main.go | 1 + deploy/supervisor/deployment.yaml | 2 ++ internal/config/supervisor/config.go | 18 +++++++++++++ internal/config/supervisor/config_test.go | 26 +++++++++++++++++-- internal/config/supervisor/types.go | 8 +++++- .../supervisorconfig/tls_cert_observer.go | 19 +++++++------- .../tls_cert_observer_test.go | 11 +++++--- test/integration/supervisor_discovery_test.go | 2 +- 8 files changed, 71 insertions(+), 16 deletions(-) diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 55e35adc..b82f5026 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -113,6 +113,7 @@ func startControllers( WithController( supervisorconfig.NewTLSCertObserverController( dynamicTLSCertProvider, + cfg.NamesConfig.DefaultTLSCertificateSecret, kubeInformers.Core().V1().Secrets(), pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(), controllerlib.WithInformer, diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml index d5161d46..8f91610d 100644 --- a/deploy/supervisor/deployment.yaml +++ b/deploy/supervisor/deployment.yaml @@ -30,6 +30,8 @@ metadata: data: #@yaml/text-templated-strings pinniped.yaml: | + names: + defaultTLSCertificateSecret: (@= defaultResourceNameWithSuffix("default-tls-certificate") @) labels: (@= json.encode(labels()).rstrip() @) --- #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "": diff --git a/internal/config/supervisor/config.go b/internal/config/supervisor/config.go index eb6836c6..c831c71e 100644 --- a/internal/config/supervisor/config.go +++ b/internal/config/supervisor/config.go @@ -8,8 +8,11 @@ package supervisor import ( "fmt" "io/ioutil" + "strings" "sigs.k8s.io/yaml" + + "go.pinniped.dev/internal/constable" ) // FromPath loads an Config from a provided local file path, inserts any @@ -30,5 +33,20 @@ func FromPath(path string) (*Config, error) { config.Labels = make(map[string]string) } + if err := validateNames(&config.NamesConfig); err != nil { + return nil, fmt.Errorf("validate names: %w", err) + } + return &config, nil } + +func validateNames(names *NamesConfigSpec) error { + missingNames := []string{} + if names.DefaultTLSCertificateSecret == "" { + missingNames = append(missingNames, "defaultTLSCertificateSecret") + } + if len(missingNames) > 0 { + return constable.Error("missing required names: " + strings.Join(missingNames, ", ")) + } + return nil +} diff --git a/internal/config/supervisor/config_test.go b/internal/config/supervisor/config_test.go index b616fa5b..8b77c6f2 100644 --- a/internal/config/supervisor/config_test.go +++ b/internal/config/supervisor/config_test.go @@ -18,6 +18,7 @@ func TestFromPath(t *testing.T) { name string yaml string wantConfig *Config + wantError string }{ { name: "Happy", @@ -26,23 +27,40 @@ func TestFromPath(t *testing.T) { labels: myLabelKey1: myLabelValue1 myLabelKey2: myLabelValue2 + names: + defaultTLSCertificateSecret: my-secret-name `), wantConfig: &Config{ Labels: map[string]string{ "myLabelKey1": "myLabelValue1", "myLabelKey2": "myLabelValue2", }, + NamesConfig: NamesConfigSpec{ + DefaultTLSCertificateSecret: "my-secret-name", + }, }, }, { name: "When only the required fields are present, causes other fields to be defaulted", yaml: here.Doc(` --- + names: + defaultTLSCertificateSecret: my-secret-name `), wantConfig: &Config{ Labels: map[string]string{}, + NamesConfig: NamesConfigSpec{ + DefaultTLSCertificateSecret: "my-secret-name", + }, }, }, + { + name: "Missing defaultTLSCertificateSecret name", + yaml: here.Doc(` + --- + `), + wantError: "validate names: missing required names: defaultTLSCertificateSecret", + }, } for _, test := range tests { test := test @@ -62,8 +80,12 @@ func TestFromPath(t *testing.T) { // Test FromPath() config, err := FromPath(f.Name()) - require.NoError(t, err) - require.Equal(t, test.wantConfig, config) + if test.wantError != "" { + require.EqualError(t, err, test.wantError) + } else { + require.NoError(t, err) + require.Equal(t, test.wantConfig, config) + } }) } } diff --git a/internal/config/supervisor/types.go b/internal/config/supervisor/types.go index 03805956..060b88ed 100644 --- a/internal/config/supervisor/types.go +++ b/internal/config/supervisor/types.go @@ -5,5 +5,11 @@ package supervisor // Config contains knobs to setup an instance of the Pinniped Supervisor. type Config struct { - Labels map[string]string `json:"labels"` + Labels map[string]string `json:"labels"` + NamesConfig NamesConfigSpec `json:"names"` +} + +// NamesConfigSpec configures the names of some Kubernetes resources for the Supervisor. +type NamesConfigSpec struct { + DefaultTLSCertificateSecret string `json:"defaultTLSCertificateSecret"` } diff --git a/internal/controller/supervisorconfig/tls_cert_observer.go b/internal/controller/supervisorconfig/tls_cert_observer.go index e29930c6..88db4ace 100644 --- a/internal/controller/supervisorconfig/tls_cert_observer.go +++ b/internal/controller/supervisorconfig/tls_cert_observer.go @@ -18,12 +18,11 @@ import ( "go.pinniped.dev/internal/controllerlib" ) -const SecretNameForDefaultTLSCertificate = "default-tls-certificate" //nolint:gosec // this is not a hardcoded credential - type tlsCertObserverController struct { - issuerTLSCertSetter IssuerTLSCertSetter - oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer - secretInformer corev1informers.SecretInformer + issuerTLSCertSetter IssuerTLSCertSetter + defaultTLSCertificateSecretName string + oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer + secretInformer corev1informers.SecretInformer } type IssuerTLSCertSetter interface { @@ -33,6 +32,7 @@ type IssuerTLSCertSetter interface { func NewTLSCertObserverController( issuerTLSCertSetter IssuerTLSCertSetter, + defaultTLSCertificateSecretName string, secretInformer corev1informers.SecretInformer, oidcProviderConfigInformer v1alpha1.OIDCProviderConfigInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, @@ -41,9 +41,10 @@ func NewTLSCertObserverController( controllerlib.Config{ Name: "tls-certs-observer-controller", Syncer: &tlsCertObserverController{ - issuerTLSCertSetter: issuerTLSCertSetter, - oidcProviderConfigInformer: oidcProviderConfigInformer, - secretInformer: secretInformer, + issuerTLSCertSetter: issuerTLSCertSetter, + defaultTLSCertificateSecretName: defaultTLSCertificateSecretName, + oidcProviderConfigInformer: oidcProviderConfigInformer, + secretInformer: secretInformer, }, }, withInformer( @@ -88,7 +89,7 @@ func (c *tlsCertObserverController) Sync(ctx controllerlib.Context) error { klog.InfoS("tlsCertObserverController Sync updated the TLS cert cache", "issuerHostCount", len(issuerHostToTLSCertMap)) c.issuerTLSCertSetter.SetIssuerHostToTLSCertMap(issuerHostToTLSCertMap) - defaultCert, err := c.certFromSecret(ns, SecretNameForDefaultTLSCertificate) + defaultCert, err := c.certFromSecret(ns, c.defaultTLSCertificateSecretName) if err != nil { c.issuerTLSCertSetter.SetDefaultTLSCert(nil) } else { diff --git a/internal/controller/supervisorconfig/tls_cert_observer_test.go b/internal/controller/supervisorconfig/tls_cert_observer_test.go index 633bad04..9b77fa46 100644 --- a/internal/controller/supervisorconfig/tls_cert_observer_test.go +++ b/internal/controller/supervisorconfig/tls_cert_observer_test.go @@ -42,6 +42,7 @@ func TestTLSCertObserverControllerInformerFilters(t *testing.T) { oidcProviderConfigInformer := pinnipedinformers.NewSharedInformerFactory(nil, 0).Config().V1alpha1().OIDCProviderConfigs() _ = NewTLSCertObserverController( nil, + "", // don't care about the secret name for this test secretsInformer, oidcProviderConfigInformer, observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters @@ -115,7 +116,10 @@ func (f *fakeIssuerTLSCertSetter) SetDefaultTLSCert(certificate *tls.Certificate func TestTLSCertObserverControllerSync(t *testing.T) { spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { - const installedInNamespace = "some-namespace" + const ( + installedInNamespace = "some-namespace" + defaultTLSSecretName = "some-default-secret-name" + ) var ( r *require.Assertions @@ -136,6 +140,7 @@ func TestTLSCertObserverControllerSync(t *testing.T) { // Set this at the last second to allow for injection of server override. subject = NewTLSCertObserverController( issuerTLSCertSetter, + defaultTLSSecretName, kubeInformers.Core().V1().Secrets(), pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(), controllerlib.WithInformer, @@ -324,7 +329,7 @@ func TestTLSCertObserverControllerSync(t *testing.T) { r.Equal(expectedCertificate2, *actualCertificate2) }) - when("there is also a default TLS cert secret called default-tls-certificate", func() { + when("there is also a default TLS cert secret with the configured default TLS cert secret name", func() { var ( expectedDefaultCertificate tls.Certificate ) @@ -338,7 +343,7 @@ func TestTLSCertObserverControllerSync(t *testing.T) { expectedDefaultCertificate, err = tls.X509KeyPair(testCrt, testKey) r.NoError(err) defaultTLSCertSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "default-tls-certificate", Namespace: installedInNamespace}, + ObjectMeta: metav1.ObjectMeta{Name: defaultTLSSecretName, Namespace: installedInNamespace}, Data: map[string][]byte{"tls.crt": testCrt, "tls.key": testKey}, } r.NoError(kubeInformerClient.Tracker().Add(defaultTLSCertSecret)) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index ec1d0f09..45fefcdf 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -135,7 +135,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts(t *testing.T) { requireEndpointHasTLSErrorBecauseCertificatesAreNotReady(t, issuerUsingIPAddress) // Create a Secret at the special name which represents the default TLS cert. - specialNameForDefaultTLSCertSecret := "default-tls-certificate" //nolint:gosec // this is not a hardcoded credential + specialNameForDefaultTLSCertSecret := "pinniped-supervisor-default-tls-certificate" //nolint:gosec // this is not a hardcoded credential defaultCA := createTLSCertificateSecret(ctx, t, ns, "cert-hostname-doesnt-matter", []net.IP{ip}, specialNameForDefaultTLSCertSecret, kubeClient) // Now that the Secret exists, we should be able to access the endpoints by IP address using the CA.