Merge pull request #65182 from liggitt/restore-service-account-key-file-arg

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Restore help for --service-account-private-key-file flag, avoid incorrect deprecation notice

https://github.com/kubernetes/kubernetes/pull/60270/files#diff-fc9db90cc6b68d7c3ca838a8512447e9R61 incorrectly merged the ServiceAccountKeyFile flag used by the cloud controller manager (deprecated and scheduled to be removed in 1.10), and the one used by the kube-controller-manager (not deprecated, still supported)

This made the help for the supported flag disappear from 1.11 help output.

This PR:
* moves the option to the service account controller options
* removes the incorrect deprecation notice
* drops the option from the cloud-controller-manager

```release-note
The deprecated `--service-account-private-key-file` flag has been removed from the cloud-controller-manager. The flag is still present and supported in the kube-controller-manager.
```

cmd/controller-manager/app/options/kubecloudshared.go

@@ -36,7 +36,6 @@ type KubeCloudSharedOptions struct {
 	AllocateNodeCIDRs            bool
 	CIDRAllocatorType            string
 	ConfigureCloudRoutes         bool
-	ServiceAccountKeyFile        string
 	NodeSyncPeriod               metav1.Duration
 }
 
@@ -74,9 +73,6 @@ func (o *KubeCloudSharedOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&o.CIDRAllocatorType, "cidr-allocator-type", "RangeAllocator", "Type of CIDR allocator to use")
 	fs.BoolVar(&o.ConfigureCloudRoutes, "configure-cloud-routes", true, "Should CIDRs allocated by allocate-node-cidrs be configured on the cloud provider.")
 
-	// TODO: remove --service-account-private-key-file 6 months after 1.8 is released (~1.10)
-	fs.StringVar(&o.ServiceAccountKeyFile, "service-account-private-key-file", o.ServiceAccountKeyFile, "Filename containing a PEM-encoded private RSA or ECDSA key used to sign service account tokens.")
-	fs.MarkDeprecated("service-account-private-key-file", "This flag is currently no-op and will be deleted.")
 	fs.DurationVar(&o.NodeSyncPeriod.Duration, "node-sync-period", 0, ""+
 		"This flag is deprecated and will be removed in future releases. See node-monitor-period for Node health checking or "+
 		"route-reconciliation-period for cloud provider's route configuration settings.")
@@ -100,7 +96,6 @@ func (o *KubeCloudSharedOptions) ApplyTo(cfg *componentconfig.KubeCloudSharedCon
 	cfg.AllocateNodeCIDRs = o.AllocateNodeCIDRs
 	cfg.CIDRAllocatorType = o.CIDRAllocatorType
 	cfg.ConfigureCloudRoutes = o.ConfigureCloudRoutes
-	cfg.ServiceAccountKeyFile = o.ServiceAccountKeyFile
 	cfg.NodeSyncPeriod = o.NodeSyncPeriod
 
 	return nil

cmd/controller-manager/app/options/serviceaccountcontroller.go

@@ -23,6 +23,7 @@ import (
 
 // SAControllerOptions holds the ServiceAccountController options.
 type SAControllerOptions struct {
+	ServiceAccountKeyFile  string
 	ConcurrentSATokenSyncs int32
 	RootCAFile             string
 }
@@ -33,6 +34,7 @@ func (o *SAControllerOptions) AddFlags(fs *pflag.FlagSet) {
 		return
 	}
 
+	fs.StringVar(&o.ServiceAccountKeyFile, "service-account-private-key-file", o.ServiceAccountKeyFile, "Filename containing a PEM-encoded private RSA or ECDSA key used to sign service account tokens.")
 	fs.Int32Var(&o.ConcurrentSATokenSyncs, "concurrent-serviceaccount-token-syncs", o.ConcurrentSATokenSyncs, "The number of service account token objects that are allowed to sync concurrently. Larger number = more responsive token generation, but more CPU (and network) load")
 	fs.StringVar(&o.RootCAFile, "root-ca-file", o.RootCAFile, "If set, this root certificate authority will be included in service account's token secret. This must be a valid PEM-encoded CA bundle.")
 }
@@ -43,6 +45,7 @@ func (o *SAControllerOptions) ApplyTo(cfg *componentconfig.SAControllerConfigura
 		return nil
 	}
 
+	cfg.ServiceAccountKeyFile = o.ServiceAccountKeyFile
 	cfg.ConcurrentSATokenSyncs = o.ConcurrentSATokenSyncs
 	cfg.RootCAFile = o.RootCAFile
 

cmd/kube-controller-manager/app/controllermanager.go

@@ -150,7 +150,7 @@ func Run(c *config.CompletedConfig) error {
 		}
 		var clientBuilder controller.ControllerClientBuilder
 		if c.ComponentConfig.KubeCloudShared.UseServiceAccountCredentials {
-			if len(c.ComponentConfig.KubeCloudShared.ServiceAccountKeyFile) == 0 {
+			if len(c.ComponentConfig.SAController.ServiceAccountKeyFile) == 0 {
 				// It'c possible another controller process is creating the tokens for us.
 				// If one isn't, we'll timeout and exit when our client builder is unable to create the tokens.
 				glog.Warningf("--use-service-account-credentials was specified without providing a --service-account-private-key-file")
@@ -484,11 +484,11 @@ func (c serviceAccountTokenControllerStarter) startServiceAccountTokenController
 		return false, nil
 	}
 
-	if len(ctx.ComponentConfig.KubeCloudShared.ServiceAccountKeyFile) == 0 {
+	if len(ctx.ComponentConfig.SAController.ServiceAccountKeyFile) == 0 {
 		glog.Warningf("%q is disabled because there is no private key", saTokenControllerName)
 		return false, nil
 	}
-	privateKey, err := certutil.PrivateKeyFromFile(ctx.ComponentConfig.KubeCloudShared.ServiceAccountKeyFile)
+	privateKey, err := certutil.PrivateKeyFromFile(ctx.ComponentConfig.SAController.ServiceAccountKeyFile)
 	if err != nil {
 		return true, fmt.Errorf("error reading key for service account token controller: %v", err)
 	}

cmd/kube-controller-manager/app/options/options_test.go

@@ -149,7 +149,6 @@ func TestAddFlags(t *testing.T) {
 			AllocateNodeCIDRs:            true,
 			CIDRAllocatorType:            "CloudAllocator",
 			ConfigureCloudRoutes:         false,
-			ServiceAccountKeyFile:        "/service-account-private-key",
 		},
 		AttachDetachController: &cmoptions.AttachDetachControllerOptions{
 			ReconcilerSyncLoopPeriod:          metav1.Duration{Duration: 30 * time.Second},
@@ -247,6 +246,7 @@ func TestAddFlags(t *testing.T) {
 			ConcurrentResourceQuotaSyncs: 10,
 		},
 		SAController: &cmoptions.SAControllerOptions{
+			ServiceAccountKeyFile:  "/service-account-private-key",
 			ConcurrentSATokenSyncs: 10,
 		},
 		ServiceController: &cmoptions.ServiceControllerOptions{

pkg/apis/componentconfig/types.go

@@ -323,9 +323,6 @@ type KubeCloudSharedConfiguration struct {
 	// configureCloudRoutes enables CIDRs allocated with allocateNodeCIDRs
 	// to be configured on the cloud provider.
 	ConfigureCloudRoutes bool
-	// serviceAccountKeyFile is the filename containing a PEM-encoded private RSA key
-	// used to sign service account tokens.
-	ServiceAccountKeyFile string
 	// nodeSyncPeriod is the period for syncing nodes from cloudprovider. Longer
 	// periods will result in fewer calls to cloud provider, but may delay addition
 	// of new nodes to cluster.
@@ -506,6 +503,9 @@ type ResourceQuotaControllerConfiguration struct {
 }
 
 type SAControllerConfiguration struct {
+	// serviceAccountKeyFile is the filename containing a PEM-encoded private RSA key
+	// used to sign service account tokens.
+	ServiceAccountKeyFile string
 	// concurrentSATokenSyncs is the number of service account token syncing operations
 	// that will be done concurrently.
 	ConcurrentSATokenSyncs int32

pkg/apis/componentconfig/v1alpha1/types.go

@@ -366,9 +366,6 @@ type KubeCloudSharedConfiguration struct {
 	// configureCloudRoutes enables CIDRs allocated with allocateNodeCIDRs
 	// to be configured on the cloud provider.
 	ConfigureCloudRoutes *bool `json:"configureCloudRoutes"`
-	// serviceAccountKeyFile is the filename containing a PEM-encoded private RSA key
-	// used to sign service account tokens.
-	ServiceAccountKeyFile string `json:"serviceAccountKeyFile"`
 	// nodeSyncPeriod is the period for syncing nodes from cloudprovider. Longer
 	// periods will result in fewer calls to cloud provider, but may delay addition
 	// of new nodes to cluster.
@@ -549,6 +546,9 @@ type ResourceQuotaControllerConfiguration struct {
 }
 
 type SAControllerConfiguration struct {
+	// serviceAccountKeyFile is the filename containing a PEM-encoded private RSA key
+	// used to sign service account tokens.
+	ServiceAccountKeyFile string `json:"serviceAccountKeyFile"`
 	// concurrentSATokenSyncs is the number of service account token syncing operations
 	// that will be done concurrently.
 	ConcurrentSATokenSyncs int32 `json:"concurrentSATokenSyncs"`

pkg/apis/componentconfig/v1alpha1/zz_generated.conversion.go

@@ -516,7 +516,6 @@ func autoConvert_v1alpha1_KubeCloudSharedConfiguration_To_componentconfig_KubeCl
 	if err := v1.Convert_Pointer_bool_To_bool(&in.ConfigureCloudRoutes, &out.ConfigureCloudRoutes, s); err != nil {
 		return err
 	}
-	out.ServiceAccountKeyFile = in.ServiceAccountKeyFile
 	out.NodeSyncPeriod = in.NodeSyncPeriod
 	return nil
 }
@@ -540,7 +539,6 @@ func autoConvert_componentconfig_KubeCloudSharedConfiguration_To_v1alpha1_KubeCl
 	if err := v1.Convert_bool_To_Pointer_bool(&in.ConfigureCloudRoutes, &out.ConfigureCloudRoutes, s); err != nil {
 		return err
 	}
-	out.ServiceAccountKeyFile = in.ServiceAccountKeyFile
 	out.NodeSyncPeriod = in.NodeSyncPeriod
 	return nil
 }
@@ -1045,6 +1043,7 @@ func Convert_componentconfig_ResourceQuotaControllerConfiguration_To_v1alpha1_Re
 }
 
 func autoConvert_v1alpha1_SAControllerConfiguration_To_componentconfig_SAControllerConfiguration(in *SAControllerConfiguration, out *componentconfig.SAControllerConfiguration, s conversion.Scope) error {
+	out.ServiceAccountKeyFile = in.ServiceAccountKeyFile
 	out.ConcurrentSATokenSyncs = in.ConcurrentSATokenSyncs
 	out.RootCAFile = in.RootCAFile
 	return nil
@@ -1056,6 +1055,7 @@ func Convert_v1alpha1_SAControllerConfiguration_To_componentconfig_SAControllerC
 }
 
 func autoConvert_componentconfig_SAControllerConfiguration_To_v1alpha1_SAControllerConfiguration(in *componentconfig.SAControllerConfiguration, out *SAControllerConfiguration, s conversion.Scope) error {
+	out.ServiceAccountKeyFile = in.ServiceAccountKeyFile
 	out.ConcurrentSATokenSyncs = in.ConcurrentSATokenSyncs
 	out.RootCAFile = in.RootCAFile
 	return nil