Skip to content

Commit

Permalink
feat: add a preflight check to prevent provider downgrades
Browse files Browse the repository at this point in the history
Cluster API doesn't support provider downgrades due to potential
breaking changes in CRDs. Currently, if users try to downgrade
for example from v1.6.0 to v1.5.2, they get a error:
"invalid provider metadata: version v1.6.1 (the current version) for the provider capi-system/cluster-api does not match any release series".

This is not really informative, so it is better to add a preflight
check to fail earlier and provide more descriptive error messages.
  • Loading branch information
Fedosin committed Jan 25, 2024
1 parent f5d8954 commit a22f799
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 11 deletions.
3 changes: 3 additions & 0 deletions api/v1alpha2/conditions_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const (

// NoDeploymentAvailableConditionReason documents that there is no Available condition for provider deployment yet.
NoDeploymentAvailableConditionReason = "NoDeploymentAvailableConditionReason"

// UnsupportedProviderDowngradeReason documents that the provider downgrade is not supported.
UnsupportedProviderDowngradeReason = "UnsupportedProviderDowngradeReason"
)

const (
Expand Down
55 changes: 44 additions & 11 deletions internal/controller/preflight_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ var (
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"
unsupportedProviderDowngradeMessage = "Downgrade is not supported for provider %s"
)

// preflightChecks performs preflight checks before installing provider.
Expand All @@ -58,18 +59,10 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi

spec := provider.GetSpec()

// Check that provider version contains a valid value if it's not empty.
if spec.Version != "" {
if _, err := version.ParseSemantic(spec.Version); err != nil {
log.Info("Version contains invalid value")
conditions.Set(provider, conditions.FalseCondition(
operatorv1.PreflightCheckCondition,
operatorv1.IncorrectVersionFormatReason,
clusterv1.ConditionSeverityError,
err.Error(),
))

return ctrl.Result{}, fmt.Errorf("version contains invalid value for provider %q", provider.GetName())
// Check that the provider version is supported.
if err := checkProviderVersion(ctx, spec.Version, provider); err != nil {
return ctrl.Result{}, err
}
}

Expand Down Expand Up @@ -208,6 +201,46 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
return ctrl.Result{}, nil
}

// checkProviderVersion verifies that target and installed provider versions are correct.
func checkProviderVersion(ctx context.Context, providerVersion string, provider genericprovider.GenericProvider) error {
log := ctrl.LoggerFrom(ctx)

// Check that provider version contains a valid value if it's not empty.
targetVersion, err := version.ParseSemantic(providerVersion)
if err != nil {
log.Info("Version contains invalid value")
conditions.Set(provider, conditions.FalseCondition(
operatorv1.PreflightCheckCondition,
operatorv1.IncorrectVersionFormatReason,
clusterv1.ConditionSeverityError,
err.Error(),
))

return fmt.Errorf("version contains invalid value for provider %q", provider.GetName())
}

// Cluster API doesn't support downgrades by design. We need to report that for the user.
if provider.GetStatus().InstalledVersion != nil && *provider.GetStatus().InstalledVersion != "" {
installedVersion, err := version.ParseSemantic(*provider.GetStatus().InstalledVersion)
if err != nil {
return fmt.Errorf("installed version contains invalid value for provider %q", provider.GetName())
}

if targetVersion.Major() < installedVersion.Major() || targetVersion.Major() == installedVersion.Major() && targetVersion.Minor() < installedVersion.Minor() {
conditions.Set(provider, conditions.FalseCondition(
operatorv1.PreflightCheckCondition,
operatorv1.UnsupportedProviderDowngradeReason,
clusterv1.ConditionSeverityError,
fmt.Sprintf(unsupportedProviderDowngradeMessage, provider.GetName(), configclient.ClusterAPIProviderName),
))

return fmt.Errorf("downgrade is not supported for provider %q", provider.GetName())
}
}

return nil
}

// coreProviderIsReady returns true if the core provider is ready.
func coreProviderIsReady(ctx context.Context, c client.Client) (bool, error) {
cpl := &operatorv1.CoreProviderList{}
Expand Down
100 changes: 100 additions & 0 deletions internal/controller/preflight_checks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,103 @@ func TestPreflightChecks(t *testing.T) {
})
}
}

func TestPreflightChecksUpgradesDowngrades(t *testing.T) {
testCases := []struct {
name string
installedVersion string
targetVersion string
expectedConditionStatus corev1.ConditionStatus
expectedError bool
}{
{
name: "upgrade core provider major version",
expectedConditionStatus: corev1.ConditionTrue,
installedVersion: "v1.9.0",
targetVersion: "v2.0.0",
},
{
name: "upgrade core provider minor version",
expectedConditionStatus: corev1.ConditionTrue,
installedVersion: "v1.9.0",
targetVersion: "v1.10.0",
},
{
name: "downgrade core provider major version",
expectedConditionStatus: corev1.ConditionFalse,
installedVersion: "v2.0.0",
targetVersion: "v1.9.0",
expectedError: true,
},
{
name: "downgrade core provider minor version",
expectedConditionStatus: corev1.ConditionFalse,
installedVersion: "v1.10.0",
targetVersion: "v1.9.0",
expectedError: true,
},
{
name: "downgrade core provider patch version",
expectedConditionStatus: corev1.ConditionTrue,
installedVersion: "v1.10.1",
targetVersion: "v1.10.0",
},
{
name: "same version",
expectedConditionStatus: corev1.ConditionTrue,
installedVersion: "v1.10.0",
targetVersion: "v1.10.0",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
gs := NewWithT(t)

provider := &operatorv1.CoreProvider{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-api",
Namespace: "provider-test-ns-1",
},
TypeMeta: metav1.TypeMeta{
Kind: "CoreProvider",
APIVersion: "operator.cluster.x-k8s.io/v1alpha1",
},
Spec: operatorv1.CoreProviderSpec{
ProviderSpec: operatorv1.ProviderSpec{
Version: tc.targetVersion,
FetchConfig: &operatorv1.FetchConfiguration{
URL: "https://example.com",
},
},
},
Status: operatorv1.CoreProviderStatus{
ProviderStatus: operatorv1.ProviderStatus{
InstalledVersion: &tc.installedVersion,
},
},
}

fakeclient := fake.NewClientBuilder().WithObjects().Build()

gs.Expect(fakeclient.Create(ctx, provider)).To(Succeed())

_, err := preflightChecks(context.Background(), fakeclient, provider, &operatorv1.CoreProviderList{})
if tc.expectedError {
gs.Expect(err).To(HaveOccurred())
} else {
gs.Expect(err).ToNot(HaveOccurred())
}

// Check if proper condition is returned
gs.Expect(provider.GetStatus().Conditions).To(HaveLen(1))
gs.Expect(provider.GetStatus().Conditions[0].Type).To(Equal(operatorv1.PreflightCheckCondition))
gs.Expect(provider.GetStatus().Conditions[0].Status).To(Equal(tc.expectedConditionStatus))

if tc.expectedConditionStatus == corev1.ConditionFalse {
gs.Expect(provider.GetStatus().Conditions[0].Reason).To(Equal(operatorv1.UnsupportedProviderDowngradeReason))
gs.Expect(provider.GetStatus().Conditions[0].Severity).To(Equal(clusterv1.ConditionSeverityError))
}
})
}
}

0 comments on commit a22f799

Please sign in to comment.