Repository: kubernetes/kubernetes (language: go)

Issue:
Cloud provider detection functions are innaccurate and could lead to undefined behavior
### What happened?

During the course of the work for [KEP-2395 (Removing in-tree Cloud Providers)](https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/2395-removing-in-tree-cloud-providers) the internal cloud provider loops have been removed but some of the warning and detection helper functions are no longer accurate and could theoretically cause undefined behaviors or panics.

For example, this function for detecting if a provider is deprecated:
```go
func IsDeprecatedInternal(name string) bool {
	for _, provider := range deprecatedCloudProviders {
		if provider.name == name {
			return true
		}
	}

	return false
}
```
relies on the `deprecatedCloudProviders` containing a list, but the actual definition is empty:
```go
deprecatedCloudProviders = []struct {
		name     string
		external bool
		detail   string
	}{}
```

This is one example of a systemic issue where several commands (kubelet, kube-controller-manager, and kube-apiserver) rely on this function to gate behavior, and warnings, about the in-tree controller loops. But, given the logic, these functions will not clearly advertise the state of the removed providers.

### What did you expect to happen?

I would expect the functions related to detecting and warning about in-tree cloud provider loops to be relevant for any value of the `--cloud-provider` flag which is not `external`. The controller loops have been removed and there is technical debt surrounding the detection code that should be removed for the health of the project.

### How can we reproduce it (as minimally and precisely as possible)?

Passing an incorrect value to the `--cloud-provider` flag for kubelet, kube-controller-manager, and kube-apiserver would be the primary way to see this problem, but we have done a fairly good job in preventing bad values from making it to these internal functions.

There are some conditions underwhich the kube-controller-manager has been observed to panic when a malformed value is passed, but this is not consistent and it is not clear what combination of flags is leading to the panic.

### Anything else we need to know?

There is related code around the detection and use of the internal cloud provider loops that needs to be cleaned up. It does not appear to be negatively affecting the functioning of kubernetes, but this code is effectively not in use and could pose a security weakness as it continues to bit rot.

### Kubernetes version

<details>

1.31 is the completion of KEP-2395 and as such all the internal loops have been removed, this change should be affected from 1.31 onwards.


</details>


### Cloud provider

<details>

this affects any value of cloud provider that is not "external"
</details>


### OS version

n/a

### Install tools

n/a

### Container runtime (CRI) and version (if applicable)

n/a

### Related plugins (CNI, CSI, ...) and versions (if applicable)

n/a


Discussion / hints:
/assign elmiko
/sig cloud-provider
i am working on a patch to clean up the warning/detection functions, but i have a feeling there will need to be a followup for more housekeeping.

Fix patch:
diff --git a/cmd/kube-controller-manager/app/cloudproviders.go b/cmd/kube-controller-manager/app/cloudproviders.go
index f93890d24c449..ab259f5b17ff4 100644
--- a/cmd/kube-controller-manager/app/cloudproviders.go
+++ b/cmd/kube-controller-manager/app/cloudproviders.go
@@ -19,9 +19,7 @@ package app
 import (
 	"fmt"
 
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/features"
 
 	"k8s.io/client-go/informers"
 	cloudprovider "k8s.io/cloud-provider"
@@ -32,18 +30,10 @@ import (
 func createCloudProvider(logger klog.Logger, cloudProvider string, externalCloudVolumePlugin string, cloudConfigFile string,
 	allowUntaggedCloud bool, sharedInformers informers.SharedInformerFactory) (cloudprovider.Interface, ControllerLoopMode, error) {
 	var cloud cloudprovider.Interface
-	var loopMode ControllerLoopMode
 	var err error
-
-	if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) && cloudprovider.IsDeprecatedInternal(cloudProvider) {
-		cloudprovider.DisableWarningForProvider(cloudProvider)
-		return nil, ExternalLoops, fmt.Errorf(
-			"cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider",
-			cloudProvider)
-	}
+	loopMode := ExternalLoops
 
 	if cloudprovider.IsExternal(cloudProvider) {
-		loopMode = ExternalLoops
 		if externalCloudVolumePlugin == "" {
 			// externalCloudVolumePlugin is temporary until we split all cloud providers out.
 			// So we just tell the caller that we need to run ExternalLoops without any cloud provider.
@@ -51,10 +41,17 @@ func createCloudProvider(logger klog.Logger, cloudProvider string, externalCloud
 		}
 		cloud, err = cloudprovider.InitCloudProvider(externalCloudVolumePlugin, cloudConfigFile)
 	} else {
-		cloudprovider.DeprecationWarningForProvider(cloudProvider)
-
-		loopMode = IncludeCloudLoops
-		cloud, err = cloudprovider.InitCloudProvider(cloudProvider, cloudConfigFile)
+		// in the case where the cloudProvider is not set, we need to inform the caller that there
+		// is no cloud provider and that the default loops should be used, and there is no error.
+		// this will cause the kube-controller-manager to start the default controller contexts
+		// without being attached to a specific cloud.
+		if len(cloudProvider) == 0 {
+			loopMode = IncludeCloudLoops
+		} else {
+			// for all other cloudProvider values the internal cloud loops are disabled
+			cloudprovider.DisableWarningForProvider(cloudProvider)
+			err = cloudprovider.ErrorForDisabledProvider(cloudProvider)
+		}
 	}
 	if err != nil {
 		return nil, loopMode, fmt.Errorf("cloud provider could not be initialized: %v", err)
diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go
index 5e2300ddbb851..11bcb069a5363 100644
--- a/cmd/kubelet/app/server.go
+++ b/cmd/kubelet/app/server.go
@@ -651,16 +651,10 @@ func run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Depend
 	}
 
 	if kubeDeps.Cloud == nil {
-		if !cloudprovider.IsExternal(s.CloudProvider) {
-			cloudprovider.DeprecationWarningForProvider(s.CloudProvider)
-			cloud, err := cloudprovider.InitCloudProvider(s.CloudProvider, s.CloudConfigFile)
-			if err != nil {
-				return err
-			}
-			if cloud != nil {
-				klog.V(2).InfoS("Successfully initialized cloud provider", "cloudProvider", s.CloudProvider, "cloudConfigFile", s.CloudConfigFile)
-			}
-			kubeDeps.Cloud = cloud
+		if !cloudprovider.IsExternal(s.CloudProvider) && len(s.CloudProvider) != 0 {
+			// internal cloud provider loops are disabled
+			cloudprovider.DisableWarningForProvider(s.CloudProvider)
+			return cloudprovider.ErrorForDisabledProvider(s.CloudProvider)
 		}
 	}
 
diff --git a/pkg/kubeapiserver/options/cloudprovider.go b/pkg/kubeapiserver/options/cloudprovider.go
index 6977e8dd16bbc..5fd96aa1dbb1c 100644
--- a/pkg/kubeapiserver/options/cloudprovider.go
+++ b/pkg/kubeapiserver/options/cloudprovider.go
@@ -42,7 +42,7 @@ func (opts *CloudProviderOptions) Validate() []error {
 
 	switch {
 	case opts.CloudProvider == "":
-	case opts.CloudProvider == "external":
+	case cloudprovider.IsExternal(opts.CloudProvider):
 		if !utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) {
 			errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+
 				"please set DisableCloudProviders feature to true", opts.CloudProvider))
@@ -51,15 +51,6 @@ func (opts *CloudProviderOptions) Validate() []error {
 			errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+
 				"please set DisableKubeletCloudCredentialProviders feature to true", opts.CloudProvider))
 		}
-	case cloudprovider.IsDeprecatedInternal(opts.CloudProvider):
-		if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) {
-			errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+
-				"please set DisableCloudProviders feature to false", opts.CloudProvider))
-		}
-		if utilfeature.DefaultFeatureGate.Enabled(features.DisableKubeletCloudCredentialProviders) {
-			errs = append(errs, fmt.Errorf("when using --cloud-provider set to '%s', "+
-				"please set DisableKubeletCloudCredentialProviders feature to false", opts.CloudProvider))
-		}
 	default:
 		errs = append(errs, fmt.Errorf("unknown --cloud-provider: %s", opts.CloudProvider))
 	}
diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
index df9a401b5c075..81e9ce4c4cdad 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -386,7 +386,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		return nil, fmt.Errorf("invalid sync frequency %d", kubeCfg.SyncFrequency.Duration)
 	}
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) && cloudprovider.IsDeprecatedInternal(cloudProvider) {
+	if !cloudprovider.IsExternal(cloudProvider) && len(cloudProvider) != 0 {
 		cloudprovider.DisableWarningForProvider(cloudProvider)
 		return nil, fmt.Errorf("cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", cloudProvider)
 	}
diff --git a/staging/src/k8s.io/cloud-provider/plugins.go b/staging/src/k8s.io/cloud-provider/plugins.go
index 458fff69a8351..8ef778b17856a 100644
--- a/staging/src/k8s.io/cloud-provider/plugins.go
+++ b/staging/src/k8s.io/cloud-provider/plugins.go
@@ -33,13 +33,8 @@ type Factory func(config io.Reader) (Interface, error)
 
 // All registered cloud providers.
 var (
-	providersMutex           sync.Mutex
-	providers                = make(map[string]Factory)
-	deprecatedCloudProviders = []struct {
-		name     string
-		external bool
-		detail   string
-	}{}
+	providersMutex sync.Mutex
+	providers      = make(map[string]Factory)
 )
 
 const externalCloudProvider = "external"
@@ -85,47 +80,19 @@ func IsExternal(name string) bool {
 	return name == externalCloudProvider
 }
 
-// IsDeprecatedInternal is responsible for preventing cloud.Interface
-// from being initialized in kubelet, kube-controller-manager or kube-api-server
-func IsDeprecatedInternal(name string) bool {
-	for _, provider := range deprecatedCloudProviders {
-		if provider.name == name {
-			return true
-		}
-	}
-
-	return false
-}
-
 // DisableWarningForProvider logs information about disabled cloud provider state
 func DisableWarningForProvider(providerName string) {
-	for _, provider := range deprecatedCloudProviders {
-		if provider.name == providerName {
-			klog.Infof("INFO: Please make sure you are running external cloud controller manager binary for provider %q."+
-				"In-tree cloud providers are currently disabled. Refer to https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/cloud-provider/sample"+
-				"for example implementation.", providerName)
-			detail := fmt.Sprintf("Please reach to sig-cloud-provider and use 'external' cloud provider for %q: %s", providerName, provider.detail)
-			klog.Warningf("WARNING: %q built-in cloud provider is now disabled. %s", providerName, detail)
-			break
-		}
+	if !IsExternal(providerName) {
+		klog.Infof("INFO: Please make sure you are running an external cloud controller manager binary for provider %q."+
+			"In-tree cloud providers are disabled. Refer to https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/cloud-provider/sample "+
+			"for an example implementation.", providerName)
+		klog.Warningf("WARNING: built-in cloud providers are disabled. Please set \"--cloud-provider=external\" and migrate to an external cloud controller manager for provider %q", providerName)
 	}
 }
 
-// DeprecationWarningForProvider logs information about deprecated cloud provider state
-func DeprecationWarningForProvider(providerName string) {
-	for _, provider := range deprecatedCloudProviders {
-		if provider.name != providerName {
-			continue
-		}
-
-		detail := provider.detail
-		if provider.external {
-			detail = fmt.Sprintf("Please use 'external' cloud provider for %s: %s", providerName, provider.detail)
-		}
-
-		klog.Warningf("WARNING: %s built-in cloud provider is now deprecated. %s", providerName, detail)
-		break
-	}
+// ErrorForDisabledProvider returns an error formatted with the supplied provider name
+func ErrorForDisabledProvider(providerName string) error {
+	return fmt.Errorf("cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", providerName)
 }
 
 // InitCloudProvider creates an instance of the named cloud provider.


Test patch: