From 18ce801beac34489f313d6b1223a1a5f07200cb6 Mon Sep 17 00:00:00 2001 From: Nick Turner Date: Thu, 24 Mar 2022 03:19:35 +0000 Subject: [PATCH] Remove the deprecated insecure serving from the cloud controller manager --- .../cloud-provider/app/controllermanager.go | 9 ------ .../cloud-provider/app/testing/testserver.go | 16 ---------- .../k8s.io/cloud-provider/options/options.go | 30 ++++--------------- .../cloud-provider/options/options_test.go | 29 +++++++----------- test/integration/serving/serving_test.go | 2 +- 5 files changed, 18 insertions(+), 68 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index dd88a7c24ed..72ebde67661 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/healthz" cacheddiscovery "k8s.io/client-go/discovery/cached" "k8s.io/client-go/informers" @@ -168,14 +167,6 @@ func Run(c *cloudcontrollerconfig.CompletedConfig, cloud cloudprovider.Interface return err } } - if c.InsecureServing != nil { - unsecuredMux := genericcontrollermanager.NewBaseHandler(&c.ComponentConfig.Generic.Debugging, healthzHandler) - insecureSuperuserAuthn := server.AuthenticationInfo{Authenticator: &server.InsecureSuperuser{}} - handler := genericcontrollermanager.BuildHandlerChain(unsecuredMux, nil, &insecureSuperuserAuthn) - if err := c.InsecureServing.Serve(handler, 0, stopCh); err != nil { - return err - } - } run := func(ctx context.Context, controllerInitializers map[string]InitFunc) { clientBuilder := clientbuilder.SimpleControllerClientBuilder{ diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index bd7a7b2c8ee..35d5f0a866b 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -106,7 +106,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err commandArgs := []string{} listeners := []net.Listener{} - disableInsecure := false disableSecure := false for _, arg := range customFlags { if strings.HasPrefix(arg, "--secure-port=") { @@ -114,11 +113,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err commandArgs = append(commandArgs, arg) disableSecure = true } - } else if strings.HasPrefix(arg, "--port=") { - if arg == "--port=0" { - commandArgs = append(commandArgs, arg) - disableInsecure = true - } } else if strings.HasPrefix(arg, "--cert-dir=") { // skip it } else { @@ -137,16 +131,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err t.Logf("cloud-controller-manager will listen securely on port %d...", bindPort) } - if !disableInsecure { - listener, bindPort, err := createListenerOnFreePort() - if err != nil { - return result, fmt.Errorf("failed to create listener: %v", err) - } - listeners = append(listeners, listener) - commandArgs = append(commandArgs, fmt.Sprintf("--port=%d", bindPort)) - - t.Logf("cloud-controller-manager will listen securely on port %d...", bindPort) - } for _, listener := range listeners { listener.Close() } diff --git a/staging/src/k8s.io/cloud-provider/options/options.go b/staging/src/k8s.io/cloud-provider/options/options.go index 1371600c827..b6b7abd0770 100644 --- a/staging/src/k8s.io/cloud-provider/options/options.go +++ b/staging/src/k8s.io/cloud-provider/options/options.go @@ -51,8 +51,6 @@ import ( const ( // CloudControllerManagerUserAgent is the userAgent name when starting cloud-controller managers. CloudControllerManagerUserAgent = "cloud-controller-manager" - // DefaultInsecureCloudControllerManagerPort is the default insecure cloud-controller manager port. - DefaultInsecureCloudControllerManagerPort = 0 ) // CloudControllerManagerOptions is the main context object for the controller manager. @@ -61,11 +59,9 @@ type CloudControllerManagerOptions struct { KubeCloudShared *KubeCloudSharedOptions ServiceController *ServiceControllerOptions - SecureServing *apiserveroptions.SecureServingOptionsWithLoopback - // TODO: remove insecure serving mode - InsecureServing *apiserveroptions.DeprecatedInsecureServingOptionsWithLoopback - Authentication *apiserveroptions.DelegatingAuthenticationOptions - Authorization *apiserveroptions.DelegatingAuthorizationOptions + SecureServing *apiserveroptions.SecureServingOptionsWithLoopback + Authentication *apiserveroptions.DelegatingAuthenticationOptions + Authorization *apiserveroptions.DelegatingAuthorizationOptions Master string Kubeconfig string @@ -76,7 +72,7 @@ type CloudControllerManagerOptions struct { // NewCloudControllerManagerOptions creates a new ExternalCMServer with a default config. func NewCloudControllerManagerOptions() (*CloudControllerManagerOptions, error) { - componentConfig, err := NewDefaultComponentConfig(DefaultInsecureCloudControllerManagerPort) + componentConfig, err := NewDefaultComponentConfig() if err != nil { return nil, err } @@ -87,12 +83,7 @@ func NewCloudControllerManagerOptions() (*CloudControllerManagerOptions, error) ServiceController: &ServiceControllerOptions{ ServiceControllerConfiguration: &componentConfig.ServiceController, }, - SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), - InsecureServing: (&apiserveroptions.DeprecatedInsecureServingOptions{ - BindAddress: netutils.ParseIPSloppy(componentConfig.Generic.Address), - BindPort: int(componentConfig.Generic.Port), - BindNetwork: "tcp", - }).WithLoopback(), + SecureServing: apiserveroptions.NewSecureServingOptions().WithLoopback(), Authentication: apiserveroptions.NewDelegatingAuthenticationOptions(), Authorization: apiserveroptions.NewDelegatingAuthorizationOptions(), NodeStatusUpdateFrequency: componentConfig.NodeStatusUpdateFrequency, @@ -113,7 +104,7 @@ func NewCloudControllerManagerOptions() (*CloudControllerManagerOptions, error) } // NewDefaultComponentConfig returns cloud-controller manager configuration object. -func NewDefaultComponentConfig(insecurePort int32) (*ccmconfig.CloudControllerManagerConfiguration, error) { +func NewDefaultComponentConfig() (*ccmconfig.CloudControllerManagerConfiguration, error) { versioned := &ccmconfigv1alpha1.CloudControllerManagerConfiguration{} ccmconfigscheme.Scheme.Default(versioned) @@ -121,7 +112,6 @@ func NewDefaultComponentConfig(insecurePort int32) (*ccmconfig.CloudControllerMa if err := ccmconfigscheme.Scheme.Convert(versioned, internal, nil); err != nil { return nil, err } - internal.Generic.Port = insecurePort return internal, nil } @@ -133,7 +123,6 @@ func (o *CloudControllerManagerOptions) Flags(allControllers, disabledByDefaultC o.ServiceController.AddFlags(fss.FlagSet("service controller")) o.SecureServing.AddFlags(fss.FlagSet("secure serving")) - o.InsecureServing.AddUnqualifiedFlags(fss.FlagSet("insecure serving")) o.Authentication.AddFlags(fss.FlagSet("authentication")) o.Authorization.AddFlags(fss.FlagSet("authorization")) @@ -159,9 +148,6 @@ func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, userAgent stri if err = o.ServiceController.ApplyTo(&c.ComponentConfig.ServiceController); err != nil { return err } - if err = o.InsecureServing.ApplyTo(&c.InsecureServing, &c.LoopbackClientConfig); err != nil { - return err - } if err = o.SecureServing.ApplyTo(&c.SecureServing, &c.LoopbackClientConfig); err != nil { return err } @@ -207,9 +193,6 @@ func (o *CloudControllerManagerOptions) ApplyTo(c *config.Config, userAgent stri // sync back to component config // TODO: find more elegant way than syncing back the values. - c.ComponentConfig.Generic.Port = int32(o.InsecureServing.BindPort) - c.ComponentConfig.Generic.Address = o.InsecureServing.BindAddress.String() - c.ComponentConfig.NodeStatusUpdateFrequency = o.NodeStatusUpdateFrequency return nil @@ -223,7 +206,6 @@ func (o *CloudControllerManagerOptions) Validate(allControllers, disabledByDefau errors = append(errors, o.KubeCloudShared.Validate()...) errors = append(errors, o.ServiceController.Validate()...) errors = append(errors, o.SecureServing.Validate()...) - errors = append(errors, o.InsecureServing.Validate()...) errors = append(errors, o.Authentication.Validate()...) errors = append(errors, o.Authorization.Validate()...) diff --git a/staging/src/k8s.io/cloud-provider/options/options_test.go b/staging/src/k8s.io/cloud-provider/options/options_test.go index bc5efdba206..194f455380b 100644 --- a/staging/src/k8s.io/cloud-provider/options/options_test.go +++ b/staging/src/k8s.io/cloud-provider/options/options_test.go @@ -40,8 +40,7 @@ func TestDefaultFlags(t *testing.T) { expected := &CloudControllerManagerOptions{ Generic: &cmoptions.GenericControllerManagerConfigurationOptions{ GenericControllerManagerConfiguration: &cmconfig.GenericControllerManagerConfiguration{ - Port: DefaultInsecureCloudControllerManagerPort, // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config - Address: "0.0.0.0", // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config + Address: "0.0.0.0", MinResyncPeriod: metav1.Duration{Duration: 12 * time.Hour}, ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ ContentType: "application/vnd.kubernetes.protobuf", @@ -99,11 +98,6 @@ func TestDefaultFlags(t *testing.T) { }, HTTP2MaxStreamsPerConnection: 0, }).WithLoopback(), - InsecureServing: (&apiserveroptions.DeprecatedInsecureServingOptions{ - BindAddress: netutils.ParseIPSloppy("0.0.0.0"), - BindPort: int(0), - BindNetwork: "tcp", - }).WithLoopback(), Authentication: &apiserveroptions.DelegatingAuthenticationOptions{ CacheTTL: 10 * time.Second, TokenRequestTimeout: 10 * time.Second, @@ -136,13 +130,16 @@ func TestDefaultFlags(t *testing.T) { func TestAddFlags(t *testing.T) { fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) - s, _ := NewCloudControllerManagerOptions() + s, err := NewCloudControllerManagerOptions() + if err != nil { + t.Errorf("unexpected err: %v", err) + } + for _, f := range s.Flags([]string{""}, []string{""}).FlagSets { fs.AddFlagSet(f) } args := []string{ - "--address=192.168.4.10", "--allocate-node-cidrs=true", "--authorization-always-allow-paths=", // this proves that we can clear the default "--bind-address=192.168.4.21", @@ -168,19 +165,20 @@ func TestAddFlags(t *testing.T) { "--master=192.168.4.20", "--min-resync-period=100m", "--node-status-update-frequency=10m", - "--port=10000", "--profiling=false", "--route-reconciliation-period=30s", "--secure-port=10001", "--use-service-account-credentials=false", } - fs.Parse(args) + err = fs.Parse(args) + if err != nil { + t.Errorf("unexpected err: %v", err) + } expected := &CloudControllerManagerOptions{ Generic: &cmoptions.GenericControllerManagerConfigurationOptions{ GenericControllerManagerConfiguration: &cmconfig.GenericControllerManagerConfiguration{ - Port: DefaultInsecureCloudControllerManagerPort, // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config - Address: "0.0.0.0", // Note: InsecureServingOptions.ApplyTo will write the flag value back into the component config + Address: "0.0.0.0", MinResyncPeriod: metav1.Duration{Duration: 100 * time.Minute}, ClientConnection: componentbaseconfig.ClientConnectionConfiguration{ ContentType: "application/vnd.kubernetes.protobuf", @@ -238,11 +236,6 @@ func TestAddFlags(t *testing.T) { }, HTTP2MaxStreamsPerConnection: 47, }).WithLoopback(), - InsecureServing: (&apiserveroptions.DeprecatedInsecureServingOptions{ - BindAddress: netutils.ParseIPSloppy("192.168.4.10"), - BindPort: int(10000), - BindNetwork: "tcp", - }).WithLoopback(), Authentication: &apiserveroptions.DelegatingAuthenticationOptions{ CacheTTL: 10 * time.Second, TokenRequestTimeout: 10 * time.Second, diff --git a/test/integration/serving/serving_test.go b/test/integration/serving/serving_test.go index 689e1995818..0e2bd16e07b 100644 --- a/test/integration/serving/serving_test.go +++ b/test/integration/serving/serving_test.go @@ -164,7 +164,7 @@ users: insecureDisabled bool }{ {"kube-controller-manager", kubeControllerManagerTester{}, nil, true}, - {"cloud-controller-manager", cloudControllerManagerTester{}, []string{"--cloud-provider=fake"}, false}, + {"cloud-controller-manager", cloudControllerManagerTester{}, []string{"--cloud-provider=fake"}, true}, {"kube-scheduler", kubeSchedulerTester{}, nil, true}, }