From 88fd9e5c5eae3807fcd60df1188ba2db7a70c95b Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Tue, 19 Jan 2021 10:52:12 -0500 Subject: [PATCH] internal/config: wire API group suffix through to server components Signed-off-by: Andrew Keesler --- cmd/pinniped-supervisor/main.go | 1 + internal/apigroup/apigroup.go | 29 +++++++++++++++ internal/apigroup/apigroup_test.go | 36 +++++++++++++++++++ internal/concierge/server/server.go | 16 ++++++--- internal/config/concierge/config.go | 9 ++++- internal/config/concierge/config_test.go | 5 ++- internal/config/concierge/types.go | 3 +- internal/config/supervisor/config.go | 14 +++++++- internal/config/supervisor/config_test.go | 7 ++-- internal/config/supervisor/types.go | 9 ++--- .../controllermanager/prepare_controllers.go | 13 ++++++- 11 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 internal/apigroup/apigroup.go create mode 100644 internal/apigroup/apigroup_test.go diff --git a/cmd/pinniped-supervisor/main.go b/cmd/pinniped-supervisor/main.go index 98c9f4fe..950e07d7 100644 --- a/cmd/pinniped-supervisor/main.go +++ b/cmd/pinniped-supervisor/main.go @@ -264,6 +264,7 @@ func run(podInfo *downward.PodInfo, cfg *supervisor.Config) error { return fmt.Errorf("cannot create deployment ref: %w", err) } + _ = *cfg.APIGroupSuffix // TODO: wire API group into kubeclient. client, err := kubeclient.New(dref) if err != nil { return fmt.Errorf("cannot create k8s client: %w", err) diff --git a/internal/apigroup/apigroup.go b/internal/apigroup/apigroup.go new file mode 100644 index 00000000..065812a5 --- /dev/null +++ b/internal/apigroup/apigroup.go @@ -0,0 +1,29 @@ +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package apigroup provides centralized logic around Pinniped's API group parameterization. +package apigroup + +import ( + "fmt" + "strings" +) + +// defaultAPIGroupSuffix is the default suffix of the Concierge API group. Our generated code uses +// this suffix, so we know that we can replace this suffix with the configured API group suffix. +const defaultAPIGroupSuffix = "pinniped.dev" + +// Make constructs an API group from a baseAPIGroup and a parameterized apiGroupSuffix. +// +// We assume that all apiGroup's will end in "pinniped.dev", and therefore we can safely replace the +// reference to "pinniped.dev" with the provided apiGroupSuffix. If the provided baseAPIGroup does +// not end in "pinniped.dev", then this function will return an empty string and false. +// +// See Example_loginv1alpha1 and Example_string for more information on input/output pairs. +func Make(baseAPIGroup, apiGroupSuffix string) (string, bool) { + if !strings.HasSuffix(baseAPIGroup, defaultAPIGroupSuffix) { + return "", false + } + i := strings.LastIndex(baseAPIGroup, defaultAPIGroupSuffix) + return fmt.Sprintf("%s%s", baseAPIGroup[:i], apiGroupSuffix), true +} diff --git a/internal/apigroup/apigroup_test.go b/internal/apigroup/apigroup_test.go new file mode 100644 index 00000000..1f68c6d9 --- /dev/null +++ b/internal/apigroup/apigroup_test.go @@ -0,0 +1,36 @@ +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package apigroup + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1" +) + +func TestMakeError(t *testing.T) { + _, ok := Make("bad-suffix", "shouldnt-matter.com") + require.False(t, ok) +} + +func TestMakeSuffix(t *testing.T) { + s, ok := Make("something.pinniped.dev.something-else.pinniped.dev", "tuna.io") + require.Equal(t, "something.pinniped.dev.something-else.tuna.io", s) + require.True(t, ok) +} + +func Example_loginv1alpha1() { + s, _ := Make(loginv1alpha1.GroupName, "tuna.fish.io") + fmt.Println(s) + // Output: login.concierge.tuna.fish.io +} + +func Example_string() { + s, _ := Make("idp.supervisor.pinniped.dev", "marlin.io") + fmt.Println(s) + // Output: idp.supervisor.marlin.io +} diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index a2beccd6..fcbaa138 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -15,6 +15,7 @@ import ( genericoptions "k8s.io/apiserver/pkg/server/options" loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1" + "go.pinniped.dev/internal/apigroup" "go.pinniped.dev/internal/certauthority/dynamiccertauthority" "go.pinniped.dev/internal/concierge/apiserver" "go.pinniped.dev/internal/config/concierge" @@ -36,10 +37,6 @@ type App struct { downwardAPIPath string } -// This is ignored for now because we turn off etcd storage below, but this is -// the right prefix in case we turn it back on. -const defaultEtcdPathPrefix = "/registry/" + loginv1alpha1.GroupName - // New constructs a new App with command line args, stdout and stderr. func New(ctx context.Context, args []string, stdout, stderr io.Writer) *App { app := &App{} @@ -125,6 +122,7 @@ func (a *App) runServer(ctx context.Context) error { startControllersFunc, err := controllermanager.PrepareControllers( &controllermanager.Config{ ServerInstallationInfo: podInfo, + APIGroupSuffix: *cfg.APIGroupSuffix, NamesConfig: &cfg.NamesConfig, Labels: cfg.Labels, KubeCertAgentConfig: &cfg.KubeCertAgentConfig, @@ -146,6 +144,7 @@ func (a *App) runServer(ctx context.Context) error { authenticators, dynamiccertauthority.New(dynamicSigningCertProvider), startControllersFunc, + *cfg.APIGroupSuffix, ) if err != nil { return fmt.Errorf("could not configure aggregated API server: %w", err) @@ -167,7 +166,16 @@ func getAggregatedAPIServerConfig( authenticator credentialrequest.TokenCredentialRequestAuthenticator, issuer credentialrequest.CertIssuer, startControllersPostStartHook func(context.Context), + apiGroupSuffix string, ) (*apiserver.Config, error) { + // This is ignored for now because we turn off etcd storage below, but this is + // the right prefix in case we turn it back on. + apiGroup, ok := apigroup.Make(loginv1alpha1.GroupName, apiGroupSuffix) + if !ok { + return nil, fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, apiGroupSuffix) + } + defaultEtcdPathPrefix := fmt.Sprintf("/registry/%s", apiGroup) + recommendedOptions := genericoptions.NewRecommendedOptions( defaultEtcdPathPrefix, apiserver.Codecs.LegacyCodec(loginv1alpha1.SchemeGroupVersion), diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index 20555a7d..ead188de 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package concierge contains functionality to load/store Config's from/to @@ -40,6 +40,7 @@ func FromPath(path string) (*Config, error) { } maybeSetAPIDefaults(&config.APIConfig) + maybeSetAPIGroupSuffixDefault(&config.APIGroupSuffix) maybeSetKubeCertAgentDefaults(&config.KubeCertAgentConfig) if err := validateAPI(&config.APIConfig); err != nil { @@ -71,6 +72,12 @@ func maybeSetAPIDefaults(apiConfig *APIConfigSpec) { } } +func maybeSetAPIGroupSuffixDefault(apiGroupSuffix **string) { + if *apiGroupSuffix == nil { + *apiGroupSuffix = stringPtr("pinniped.dev") + } +} + func maybeSetKubeCertAgentDefaults(cfg *KubeCertAgentSpec) { if cfg.NamePrefix == nil { cfg.NamePrefix = stringPtr("pinniped-kube-cert-agent-") diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index 38315d74..bbba174d 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package concierge @@ -30,6 +30,7 @@ func TestFromPath(t *testing.T) { servingCertificate: durationSeconds: 3600 renewBeforeSeconds: 2400 + apiGroupSuffix: some.suffix.com names: servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate credentialIssuer: pinniped-config @@ -53,6 +54,7 @@ func TestFromPath(t *testing.T) { RenewBeforeSeconds: int64Ptr(2400), }, }, + APIGroupSuffix: stringPtr("some.suffix.com"), NamesConfig: NamesConfigSpec{ ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate", CredentialIssuer: "pinniped-config", @@ -82,6 +84,7 @@ func TestFromPath(t *testing.T) { DiscoveryInfo: DiscoveryInfoSpec{ URL: nil, }, + APIGroupSuffix: stringPtr("pinniped.dev"), APIConfig: APIConfigSpec{ ServingCertificateConfig: ServingCertificateConfigSpec{ DurationSeconds: int64Ptr(60 * 60 * 24 * 365), // about a year diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index fc8517b5..1f402a34 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package concierge @@ -9,6 +9,7 @@ import "go.pinniped.dev/internal/plog" type Config struct { DiscoveryInfo DiscoveryInfoSpec `json:"discovery"` APIConfig APIConfigSpec `json:"api"` + APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` NamesConfig NamesConfigSpec `json:"names"` KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` Labels map[string]string `json:"labels"` diff --git a/internal/config/supervisor/config.go b/internal/config/supervisor/config.go index 5c7308c5..6e20b21f 100644 --- a/internal/config/supervisor/config.go +++ b/internal/config/supervisor/config.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package supervisor contains functionality to load/store Config's from/to @@ -34,6 +34,8 @@ func FromPath(path string) (*Config, error) { config.Labels = make(map[string]string) } + maybeSetAPIGroupSuffixDefault(&config.APIGroupSuffix) + if err := validateNames(&config.NamesConfig); err != nil { return nil, fmt.Errorf("validate names: %w", err) } @@ -45,6 +47,12 @@ func FromPath(path string) (*Config, error) { return &config, nil } +func maybeSetAPIGroupSuffixDefault(apiGroupSuffix **string) { + if *apiGroupSuffix == nil { + *apiGroupSuffix = stringPtr("pinniped.dev") + } +} + func validateNames(names *NamesConfigSpec) error { missingNames := []string{} if names.DefaultTLSCertificateSecret == "" { @@ -55,3 +63,7 @@ func validateNames(names *NamesConfigSpec) error { } return nil } + +func stringPtr(s string) *string { + return &s +} diff --git a/internal/config/supervisor/config_test.go b/internal/config/supervisor/config_test.go index 8b77c6f2..a2e5d46f 100644 --- a/internal/config/supervisor/config_test.go +++ b/internal/config/supervisor/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisor @@ -24,6 +24,7 @@ func TestFromPath(t *testing.T) { name: "Happy", yaml: here.Doc(` --- + apiGroupSuffix: some.suffix.com labels: myLabelKey1: myLabelValue1 myLabelKey2: myLabelValue2 @@ -31,6 +32,7 @@ func TestFromPath(t *testing.T) { defaultTLSCertificateSecret: my-secret-name `), wantConfig: &Config{ + APIGroupSuffix: stringPtr("some.suffix.com"), Labels: map[string]string{ "myLabelKey1": "myLabelValue1", "myLabelKey2": "myLabelValue2", @@ -48,7 +50,8 @@ func TestFromPath(t *testing.T) { defaultTLSCertificateSecret: my-secret-name `), wantConfig: &Config{ - Labels: map[string]string{}, + APIGroupSuffix: stringPtr("pinniped.dev"), + Labels: map[string]string{}, NamesConfig: NamesConfigSpec{ DefaultTLSCertificateSecret: "my-secret-name", }, diff --git a/internal/config/supervisor/types.go b/internal/config/supervisor/types.go index 8a487d77..f6f17696 100644 --- a/internal/config/supervisor/types.go +++ b/internal/config/supervisor/types.go @@ -1,4 +1,4 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisor @@ -7,9 +7,10 @@ import "go.pinniped.dev/internal/plog" // Config contains knobs to setup an instance of the Pinniped Supervisor. type Config struct { - Labels map[string]string `json:"labels"` - NamesConfig NamesConfigSpec `json:"names"` - LogLevel plog.LogLevel `json:"logLevel"` + APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` + Labels map[string]string `json:"labels"` + NamesConfig NamesConfigSpec `json:"names"` + LogLevel plog.LogLevel `json:"logLevel"` } // NamesConfigSpec configures the names of some Kubernetes resources for the Supervisor. diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index c70fd4cf..a7f87d6a 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -18,6 +18,7 @@ import ( loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.20/client/concierge/clientset/versioned" pinnipedinformers "go.pinniped.dev/generated/1.20/client/concierge/informers/externalversions" + "go.pinniped.dev/internal/apigroup" "go.pinniped.dev/internal/config/concierge" "go.pinniped.dev/internal/controller/apicerts" "go.pinniped.dev/internal/controller/authenticator/authncache" @@ -45,6 +46,9 @@ type Config struct { // ServerInstallationInfo provides the name of the pod in which Pinniped is running and the namespace in which Pinniped is deployed. ServerInstallationInfo *downward.PodInfo + // APIGroupSuffix is the suffix of the Pinniped API that should be targeted by these controllers. + APIGroupSuffix string + // NamesConfig comes from the Pinniped config API (see api.Config). It specifies how Kubernetes // objects should be named. NamesConfig *concierge.NamesConfigSpec @@ -85,6 +89,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { return nil, fmt.Errorf("cannot create deployment ref: %w", err) } + _ = c.APIGroupSuffix // TODO: wire API group into kubeclient. client, err := kubeclient.New(dref) if err != nil { return nil, fmt.Errorf("could not create clients for the controllers: %w", err) @@ -106,6 +111,12 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { Name: c.NamesConfig.CredentialIssuer, } + groupName, ok := apigroup.Make(loginv1alpha1.GroupName, c.APIGroupSuffix) + if !ok { + return nil, fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, c.APIGroupSuffix) + } + apiServiceName := loginv1alpha1.SchemeGroupVersion.Version + "." + groupName + // Create controller manager. controllerManager := controllerlib. NewManager(). @@ -145,7 +156,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { apicerts.NewAPIServiceUpdaterController( c.ServerInstallationInfo.Namespace, c.NamesConfig.ServingCertificateSecret, - loginv1alpha1.SchemeGroupVersion.Version+"."+loginv1alpha1.GroupName, + apiServiceName, client.Aggregation, informers.installationNamespaceK8s.Core().V1().Secrets(), controllerlib.WithInformer,