chore: add a preflight check to validate core provider name

When you try to install a Core Provider with name different from "cluster-api", clusterctl validation fails: https://github.com/kubernetes-sigs/cluster-api/blob/main/cmd/clusterctl/client/config/providers_client.go#L414-L416 After that CAPI operator puts a vague error description in the logs and doesn't add anything meaningful to the provider status. To improve it, we add a new pre-flight check to compare Core Provider name with the desired one, and fail earlier, updating the status with correct information.
kubernetes-sigs · Aug 4, 2023 · b555767 · b555767
1 parent 6b36677
commit b555767
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 12 deletions.
diff --git a/api/v1alpha1/conditions_consts.go b/api/v1alpha1/conditions_consts.go
@@ -29,6 +29,9 @@ const (
 	// IncorrectVersionFormatReason documents that the provider version is in the incorrect format.
 	IncorrectVersionFormatReason = "IncorrectVersionFormat"
 
+	// IncorrectCoreProviderNameReason documents that the provider name is incorrect.
+	IncorrectCoreProviderNameReason = "IncorrectCoreProviderNameReason"
+
 	// EmptyVersionReason documents that the provider version is in the incorrect format.
 	EmptyVersionReason = "EmptyVersionReason"
 

diff --git a/internal/controller/preflight_checks.go b/internal/controller/preflight_checks.go
@@ -46,6 +46,7 @@ var (
 	capiVersionIncompatibilityMessage            = "CAPI operator is only compatible with %s providers, detected %s for provider %s."
 	invalidGithubTokenMessage                    = "Invalid github token, please check your github token value and its permissions" //nolint:gosec
 	waitingForCoreProviderReadyMessage           = "Waiting for the core provider to be installed."
+	incorrectCoreProviderNameMessage             = "Incorrect CoreProvider name: %s. It should be %s"
 )
 
 // preflightChecks performs preflight checks before installing provider.
@@ -71,6 +72,20 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
 		}
 	}
 
+	// Ensure that the CoreProvider is called "cluster-api".
+	if util.IsCoreProvider(provider) {
+		if provider.GetName() != configclient.ClusterAPIProviderName {
+			conditions.Set(provider, conditions.FalseCondition(
+				operatorv1.PreflightCheckCondition,
+				operatorv1.IncorrectCoreProviderNameReason,
+				clusterv1.ConditionSeverityError,
+				fmt.Sprintf(incorrectCoreProviderNameMessage, provider.GetName(), configclient.ClusterAPIProviderName),
+			))
+
+			return ctrl.Result{}, fmt.Errorf("incorrect CoreProvider name: %s, it should be %s", provider.GetName(), configclient.ClusterAPIProviderName)
+		}
+	}
+
 	// Check that if a predefined provider is being installed, and if it's not - ensure that FetchConfig is specified.
 	isPredefinedProvider, err := isPredefinedProvider(provider.GetName(), util.ClusterctlProviderType(provider))
 	if err != nil {

diff --git a/internal/controller/preflight_checks_test.go b/internal/controller/preflight_checks_test.go
@@ -74,6 +74,42 @@ func TestPreflightChecks(t *testing.T) {
 				CoreProviderList: &operatorv1.CoreProviderList{},
 			},
 		},
+		{
+			name:          "core provider with incorrect name, preflight check failed",
+			expectedError: true,
+			providers: []genericprovider.GenericProvider{
+				&genericprovider.CoreProviderWrapper{
+					CoreProvider: &operatorv1.CoreProvider{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "my-fancy-cluster-api",
+							Namespace: namespaceName1,
+						},
+						TypeMeta: metav1.TypeMeta{
+							Kind:       "CoreProvider",
+							APIVersion: "operator.cluster.x-k8s.io/v1alpha1",
+						},
+						Spec: operatorv1.CoreProviderSpec{
+							ProviderSpec: operatorv1.ProviderSpec{
+								Version: "v1.0.0",
+								FetchConfig: &operatorv1.FetchConfiguration{
+									URL: "https://example.com",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedCondition: clusterv1.Condition{
+				Type:     operatorv1.PreflightCheckCondition,
+				Reason:   operatorv1.IncorrectCoreProviderNameReason,
+				Severity: clusterv1.ConditionSeverityError,
+				Message:  "Incorrect CoreProvider name: my-fancy-cluster-api. It should be cluster-api",
+				Status:   corev1.ConditionFalse,
+			},
+			providerList: &genericprovider.CoreProviderListWrapper{
+				CoreProviderList: &operatorv1.CoreProviderList{},
+			},
+		},
 		{
 			name:          "two core providers were created, preflight check failed",
 			expectedError: true,
@@ -562,20 +598,20 @@ func TestPreflightChecks(t *testing.T) {
 			},
 		},
 		{
-			name:          "custom Core Provider without fetch config, preflight check failed",
+			name:          "custom Infrastructure Provider without fetch config, preflight check failed",
 			expectedError: true,
 			providers: []genericprovider.GenericProvider{
-				&genericprovider.CoreProviderWrapper{
-					CoreProvider: &operatorv1.CoreProvider{
+				&genericprovider.InfrastructureProviderWrapper{
+					InfrastructureProvider: &operatorv1.InfrastructureProvider{
 						ObjectMeta: metav1.ObjectMeta{
-							Name:      "my-custom-cluster-api",
+							Name:      "my-custom-aws",
 							Namespace: namespaceName1,
 						},
 						TypeMeta: metav1.TypeMeta{
-							Kind:       "CoreProvider",
+							Kind:       "InfrastructureProvider",
 							APIVersion: "operator.cluster.x-k8s.io/v1alpha1",
 						},
-						Spec: operatorv1.CoreProviderSpec{
+						Spec: operatorv1.InfrastructureProviderSpec{
 							ProviderSpec: operatorv1.ProviderSpec{
 								Version: "v1.0.0",
 							},
@@ -595,20 +631,20 @@ func TestPreflightChecks(t *testing.T) {
 			},
 		},
 		{
-			name:          "custom Core Provider with fetch config with empty values, preflight check failed",
+			name:          "custom Infrastructure Provider with fetch config with empty values, preflight check failed",
 			expectedError: true,
 			providers: []genericprovider.GenericProvider{
-				&genericprovider.CoreProviderWrapper{
-					CoreProvider: &operatorv1.CoreProvider{
+				&genericprovider.InfrastructureProviderWrapper{
+					InfrastructureProvider: &operatorv1.InfrastructureProvider{
 						ObjectMeta: metav1.ObjectMeta{
-							Name:      "my-custom-cluster-api",
+							Name:      "my-custom-aws",
 							Namespace: namespaceName1,
 						},
 						TypeMeta: metav1.TypeMeta{
-							Kind:       "CoreProvider",
+							Kind:       "InfrastructureProvider",
 							APIVersion: "operator.cluster.x-k8s.io/v1alpha1",
 						},
-						Spec: operatorv1.CoreProviderSpec{
+						Spec: operatorv1.InfrastructureProviderSpec{
 							ProviderSpec: operatorv1.ProviderSpec{
 								Version: "v1.0.0",
 								FetchConfig: &operatorv1.FetchConfiguration{