diff --git a/.gitignore b/.gitignore index 1a996a93..c86de273 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ *.swp *~ _output +apiserver.local.config/ .idea/ diff --git a/pkg/cmd/builder.go b/pkg/cmd/builder.go index e85348a4..16cdc129 100644 --- a/pkg/cmd/builder.go +++ b/pkg/cmd/builder.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/pflag" apimeta "k8s.io/apimachinery/pkg/api/meta" + utilerrors "k8s.io/apimachinery/pkg/util/errors" openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/discovery" @@ -35,7 +36,7 @@ import ( openapicommon "k8s.io/kube-openapi/pkg/common" "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver" - "sigs.k8s.io/custom-metrics-apiserver/pkg/cmd/server" + "sigs.k8s.io/custom-metrics-apiserver/pkg/cmd/options" "sigs.k8s.io/custom-metrics-apiserver/pkg/dynamicmapper" generatedcore "sigs.k8s.io/custom-metrics-apiserver/pkg/generated/openapi/core" generatedcustommetrics "sigs.k8s.io/custom-metrics-apiserver/pkg/generated/openapi/custommetrics" @@ -56,7 +57,7 @@ import ( // Methods on this struct are not safe to call from multiple goroutines without // external synchronization. type AdapterBase struct { - *server.CustomMetricsAdapterServerOptions + *options.CustomMetricsAdapterServerOptions // Name is the name of the API server. It defaults to custom-metrics-adapter Name string @@ -97,14 +98,10 @@ func (b *AdapterBase) InstallFlags() { b.initFlagSet() b.flagOnce.Do(func() { if b.CustomMetricsAdapterServerOptions == nil { - b.CustomMetricsAdapterServerOptions = server.NewCustomMetricsAdapterServerOptions() + b.CustomMetricsAdapterServerOptions = options.NewCustomMetricsAdapterServerOptions() } - b.SecureServing.AddFlags(b.FlagSet) - b.Authentication.AddFlags(b.FlagSet) - b.Authorization.AddFlags(b.FlagSet) - b.Audit.AddFlags(b.FlagSet) - b.Features.AddFlags(b.FlagSet) + b.CustomMetricsAdapterServerOptions.AddFlags(b.FlagSet) b.FlagSet.StringVar(&b.RemoteKubeConfigFile, "lister-kubeconfig", b.RemoteKubeConfigFile, "kubeconfig file pointing at the 'core' kubernetes server with enough rights to list "+ @@ -265,11 +262,18 @@ func (b *AdapterBase) Config() (*apiserver.Config, error) { } b.CustomMetricsAdapterServerOptions.OpenAPIConfig = b.OpenAPIConfig - config, err := b.CustomMetricsAdapterServerOptions.Config() + if errList := b.CustomMetricsAdapterServerOptions.Validate(); len(errList) > 0 { + return nil, utilerrors.NewAggregate(errList) + } + + serverConfig := genericapiserver.NewConfig(apiserver.Codecs) + err := b.CustomMetricsAdapterServerOptions.ApplyTo(serverConfig) if err != nil { return nil, err } - b.config = config + b.config = &apiserver.Config{ + GenericConfig: serverConfig, + } } return b.config, nil diff --git a/pkg/cmd/server/start.go b/pkg/cmd/options/options.go similarity index 61% rename from pkg/cmd/server/start.go rename to pkg/cmd/options/options.go index ab8373f3..449febf5 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/options/options.go @@ -14,31 +14,36 @@ See the License for the specific language governing permissions and limitations under the License. */ -package server +// Package options provides configuration options for the metrics API server. +package options import ( "fmt" "net" + "github.com/spf13/pflag" + genericapiserver "k8s.io/apiserver/pkg/server" genericoptions "k8s.io/apiserver/pkg/server/options" openapicommon "k8s.io/kube-openapi/pkg/common" - - "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver" ) +// CustomMetricsAdapterServerOptions contains the of options used to configure +// the metrics API server. +// +// It is based on a subset of [genericoptions.RecommendedOptions]. type CustomMetricsAdapterServerOptions struct { - // genericoptions.ReccomendedOptions - EtcdOptions SecureServing *genericoptions.SecureServingOptionsWithLoopback Authentication *genericoptions.DelegatingAuthenticationOptions Authorization *genericoptions.DelegatingAuthorizationOptions Audit *genericoptions.AuditOptions Features *genericoptions.FeatureOptions - // OpenAPIConfig OpenAPIConfig *openapicommon.Config } +// NewCustomMetricsAdapterServerOptions creates a new instance of +// CustomMetricsAdapterServerOptions with its default values. func NewCustomMetricsAdapterServerOptions() *CustomMetricsAdapterServerOptions { o := &CustomMetricsAdapterServerOptions{ SecureServing: genericoptions.NewSecureServingOptions().WithLoopback(), @@ -51,34 +56,44 @@ func NewCustomMetricsAdapterServerOptions() *CustomMetricsAdapterServerOptions { return o } -func (o CustomMetricsAdapterServerOptions) Validate(args []string) error { - return nil +// Validate validates CustomMetricsAdapterServerOptions +func (o CustomMetricsAdapterServerOptions) Validate() []error { + errors := []error{} + errors = append(errors, o.SecureServing.Validate()...) + errors = append(errors, o.Authentication.Validate()...) + errors = append(errors, o.Authorization.Validate()...) + errors = append(errors, o.Audit.Validate()...) + errors = append(errors, o.Features.Validate()...) + return errors } -func (o *CustomMetricsAdapterServerOptions) Complete() error { - return nil +// AddFlags adds the flags defined for the options, to the given flagset. +func (o *CustomMetricsAdapterServerOptions) AddFlags(fs *pflag.FlagSet) { + o.SecureServing.AddFlags(fs) + o.Authentication.AddFlags(fs) + o.Authorization.AddFlags(fs) + o.Audit.AddFlags(fs) + o.Features.AddFlags(fs) } -func (o CustomMetricsAdapterServerOptions) Config() (*apiserver.Config, error) { +// ApplyTo applies CustomMetricsAdapterServerOptions to the server configuration. +func (o *CustomMetricsAdapterServerOptions) ApplyTo(serverConfig *genericapiserver.Config) error { // TODO have a "real" external address (have an AdvertiseAddress?) if err := o.SecureServing.MaybeDefaultWithSelfSignedCerts("localhost", nil, []net.IP{net.ParseIP("127.0.0.1")}); err != nil { - return nil, fmt.Errorf("error creating self-signed certificates: %v", err) + return fmt.Errorf("error creating self-signed certificates: %v", err) } - serverConfig := genericapiserver.NewConfig(apiserver.Codecs) if err := o.SecureServing.ApplyTo(&serverConfig.SecureServing, &serverConfig.LoopbackClientConfig); err != nil { - return nil, err + return err } - if err := o.Authentication.ApplyTo(&serverConfig.Authentication, serverConfig.SecureServing, nil); err != nil { - return nil, err + return err } if err := o.Authorization.ApplyTo(&serverConfig.Authorization); err != nil { - return nil, err + return err } - if err := o.Audit.ApplyTo(serverConfig); err != nil { - return nil, err + return err } // enable OpenAPI schemas @@ -86,8 +101,5 @@ func (o CustomMetricsAdapterServerOptions) Config() (*apiserver.Config, error) { serverConfig.OpenAPIConfig = o.OpenAPIConfig } - config := &apiserver.Config{ - GenericConfig: serverConfig, - } - return config, nil + return nil } diff --git a/pkg/cmd/options/options_test.go b/pkg/cmd/options/options_test.go new file mode 100644 index 00000000..1973ac19 --- /dev/null +++ b/pkg/cmd/options/options_test.go @@ -0,0 +1,128 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package options + +import ( + "testing" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + + utilerrors "k8s.io/apimachinery/pkg/util/errors" + genericapiserver "k8s.io/apiserver/pkg/server" + + "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver" +) + +func TestValidate(t *testing.T) { + cases := []struct { + testName string + args []string + shouldErr bool + }{ + { + testName: "only-secure-port", + args: []string{"--secure-port=6443"}, // default is 443, which requires privileges + shouldErr: false, + }, + { + testName: "secure-port-0", + args: []string{"--secure-port=0"}, // means: "don't serve HTTPS at all" + shouldErr: false, + }, + { + testName: "invalid-secure-port", + args: []string{"--secure-port=-1"}, + shouldErr: true, + }, + { + testName: "empty-header", + args: []string{"--secure-port=6443", "--requestheader-username-headers=\" \""}, + shouldErr: true, + }, + { + testName: "invalid-audit-log-format", + args: []string{"--secure-port=6443", "--audit-log-path=file", "--audit-log-format=txt"}, + shouldErr: true, + }, + } + + for _, c := range cases { + t.Run(c.testName, func(t *testing.T) { + o := NewCustomMetricsAdapterServerOptions() + + flagSet := pflag.NewFlagSet("", pflag.PanicOnError) + o.AddFlags(flagSet) + err := flagSet.Parse(c.args) + assert.NoErrorf(t, err, "Error while parsing flags") + + errList := o.Validate() + err = utilerrors.NewAggregate(errList) + if c.shouldErr { + assert.Errorf(t, err, "Expected error while validating options") + } else { + assert.NoErrorf(t, err, "Error while validating options") + } + }) + } +} + +func TestApplyTo(t *testing.T) { + cases := []struct { + testName string + args []string + shouldErr bool + }{ + { + testName: "only-secure-port", + args: []string{"--secure-port=6443"}, // default is 443, which requires privileges + }, + { + testName: "secure-port-0", + args: []string{"--secure-port=0"}, // means: "don't serve HTTPS at all" + shouldErr: false, + }, + } + + for _, c := range cases { + t.Run(c.testName, func(t *testing.T) { + o := NewCustomMetricsAdapterServerOptions() + + // Unit tests have no Kubernetes cluster access + o.Authentication.RemoteKubeConfigFileOptional = true + o.Authorization.RemoteKubeConfigFileOptional = true + + flagSet := pflag.NewFlagSet("", pflag.PanicOnError) + o.AddFlags(flagSet) + err := flagSet.Parse(c.args) + assert.NoErrorf(t, err, "Error while parsing flags") + + serverConfig := genericapiserver.NewConfig(apiserver.Codecs) + err = o.ApplyTo(serverConfig) + + defer func() { + // Close the listener, if any + if serverConfig.SecureServing != nil && serverConfig.SecureServing.Listener != nil { + err := serverConfig.SecureServing.Listener.Close() + assert.NoError(t, err) + } + }() + + assert.NoErrorf(t, err, "Error while applying options") + }) + } +}