Validate version before running static validation

.gitignore

@@ -4,6 +4,7 @@ __pycache__
 *.pyc
 gomock_reflect_*
 /.vscode
+**/__debug_bin*
 /*.crt
 /*.key
 /*kubeconfig

pkg/frontend/changefeed.go

@@ -80,17 +80,42 @@ func (f *frontend) updateOcpVersions(docs []*api.OpenShiftVersionDocument) {
 	f.ocpVersionsMu.Lock()
 	defer f.ocpVersionsMu.Unlock()
 
+	hadCachedDefaultVersionBefore := f.defaultOcpVersion != ""
+	foundDefaultInUpdate := false
+
 	for _, doc := range docs {
 		if doc.OpenShiftVersion.Deleting || !doc.OpenShiftVersion.Properties.Enabled {
 			// https://docs.microsoft.com/en-us/azure/cosmos-db/change-feed-design-patterns#deletes
 			delete(f.enabledOcpVersions, doc.OpenShiftVersion.Properties.Version)
+			// If we're deleting the current default version, clear it
+			if doc.OpenShiftVersion.Properties.Version == f.defaultOcpVersion {
+				if f.baseLog != nil {
+					f.baseLog.Errorf("Default OpenShift version '%s' is being deleted from enabled versions", f.defaultOcpVersion)
+				}
+				f.defaultOcpVersion = ""
+			}
 		} else {
 			f.enabledOcpVersions[doc.OpenShiftVersion.Properties.Version] = doc.OpenShiftVersion
 			if doc.OpenShiftVersion.Properties.Default {
+				if f.defaultOcpVersion != "" && f.defaultOcpVersion != doc.OpenShiftVersion.Properties.Version {
+					if f.baseLog != nil {
+						f.baseLog.Warnf("Default OpenShift version changed from '%s' to '%s'", f.defaultOcpVersion, doc.OpenShiftVersion.Properties.Version)
+					}
+				}
 				f.defaultOcpVersion = doc.OpenShiftVersion.Properties.Version
+				foundDefaultInUpdate = true
 			}
 		}
 	}
+
+	// After processing all documents, check if we have a valid default version
+	if f.baseLog != nil {
+		if len(f.enabledOcpVersions) > 0 && f.defaultOcpVersion == "" {
+			f.baseLog.Errorf("No default OpenShift version is set in CosmosDB. %d enabled version(s) available but none marked as default. Clusters created without --version parameter will fail.", len(f.enabledOcpVersions))
+		} else if hadCachedDefaultVersionBefore && !foundDefaultInUpdate && f.defaultOcpVersion == "" {
+			f.baseLog.Error("Default OpenShift version was removed and no replacement was set. Clusters created without --version parameter will fail.")
+		}
+	}
 }
 
 func (f *frontend) updateFromIteratorRoleSets(ctx context.Context, ticker *time.Ticker, frontendIterator cosmosdb.PlatformWorkloadIdentityRoleSetDocumentIterator) {

pkg/frontend/openshiftcluster_preflightvalidation.go

@@ -123,19 +123,21 @@ func (f *frontend) _preflightValidation(ctx context.Context, log *logrus.Entry,
 	}
 	if isCreate {
 		converter.ToInternal(ext, oc)
-		if err = staticValidator.Static(ext, nil, f.env.Location(), f.env.Domain(), f.env.FeatureIsSet(env.FeatureRequireD2sWorkers), version.InstallArchitectureVersion, resourceID); err != nil {
+		if err := f.validateInstallVersion(ctx, oc); err != nil {
 			return api.ValidationResult{
 				Status: api.ValidationStatusFailed,
 				Error: &api.CloudErrorBody{
+					Code:    api.CloudErrorCodeInvalidParameter,
 					Message: err.Error(),
 				},
 			}
 		}
-		if err := f.validateInstallVersion(ctx, oc); err != nil {
+		ext := converter.ToExternal(oc)
+
+		if err = staticValidator.Static(ext, nil, f.env.Location(), f.env.Domain(), f.env.FeatureIsSet(env.FeatureRequireD2sWorkers), version.InstallArchitectureVersion, resourceID); err != nil {
 			return api.ValidationResult{
 				Status: api.ValidationStatusFailed,
 				Error: &api.CloudErrorBody{
-					Code:    api.CloudErrorCodeInvalidParameter,
 					Message: err.Error(),
 				},
 			}

pkg/frontend/openshiftcluster_preflightvalidation_test.go

@@ -425,6 +425,7 @@ func TestPreflightValidation(t *testing.T) {
 			fixture: func(f *testdatabase.Fixture) {
 				f.AddSubscriptionDocuments(api.ExampleSubscriptionDocument())
 			},
+			ocpVersionsChangeFeed: getOCPVersionsChangeFeed(),
 			preflightRequest: func() *api.PreflightRequest {
 				return &api.PreflightRequest{
 					Resources: []json.RawMessage{
@@ -438,6 +439,7 @@ func TestPreflightValidation(t *testing.T) {
 								"properties": {
 									"clusterProfile": {
 										"domain": "%s",
+										"version": "%s",
 										"resourceGroupId": "/subscriptions/00000000-0000-0000-0000-000000000001/resourceGroups/resourcenameTest",
 										"fipsValidatedModules": "%s"
 									},
@@ -450,7 +452,7 @@ func TestPreflightValidation(t *testing.T) {
 							`, apiVersion, clusterId,
 							api.ExampleOpenShiftClusterDocument().OpenShiftCluster.Name,
 							api.ExampleOpenShiftClusterDocument().OpenShiftCluster.Type,
-							location, defaultProfile, api.FipsValidatedModulesEnabled, mockSubID, mockSubID)),
+							location, defaultProfile, version.DefaultInstallStream.Version.String(), api.FipsValidatedModulesEnabled, mockSubID, mockSubID)),
 					},
 				}
 			},

pkg/frontend/openshiftcluster_putorpatch.go

@@ -251,6 +251,13 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus.
 
 	if isCreate {
 		putOrPatchClusterParameters.converter.ToInternal(ext, doc.OpenShiftCluster)
+
+		err = f.setDefaultVersionIfEmpty(doc.OpenShiftCluster)
+		if err != nil {
+			return nil, err
+		}
+		ext = putOrPatchClusterParameters.converter.ToExternal(doc.OpenShiftCluster)
+
 		err = f.ValidateNewCluster(ctx, subscription, doc.OpenShiftCluster, putOrPatchClusterParameters.staticValidator, ext, putOrPatchClusterParameters.path)
 		if err != nil {
 			return nil, err

pkg/frontend/validate.go

@@ -231,15 +231,15 @@ func validateAdminMasterVMSize(vmSize string) error {
 // validateInstallVersion validates the install version set in the clusterprofile.version
 // TODO convert this into static validation instead of this receiver function in the validation for frontend.
 func (f *frontend) validateInstallVersion(ctx context.Context, oc *api.OpenShiftCluster) error {
-	f.ocpVersionsMu.RLock()
-	// If this request is from an older API or the user did not specify
-	// the version to install, use the default version.
-	if oc.Properties.ClusterProfile.Version == "" {
-		oc.Properties.ClusterProfile.Version = f.defaultOcpVersion
+	if err := f.setDefaultVersionIfEmpty(oc); err != nil {
+		return err
 	}
-	_, ok := f.enabledOcpVersions[oc.Properties.ClusterProfile.Version]
-	f.ocpVersionsMu.RUnlock()
 
+	f.ocpVersionsMu.RLock()
+	defer f.ocpVersionsMu.RUnlock()
+
+	// Validate the version (whether user-provided or default)
+	_, ok := f.enabledOcpVersions[oc.Properties.ClusterProfile.Version]
 	_, err := semver.NewVersion(oc.Properties.ClusterProfile.Version)
 
 	if !ok || err != nil {
@@ -248,3 +248,20 @@ func (f *frontend) validateInstallVersion(ctx context.Context, oc *api.OpenShift
 
 	return nil
 }
+
+func (f *frontend) setDefaultVersionIfEmpty(oc *api.OpenShiftCluster) error {
+	f.ocpVersionsMu.RLock()
+	defer f.ocpVersionsMu.RUnlock()
+
+	// If user did not specify a version, use the default version from cache
+	if oc.Properties.ClusterProfile.Version == "" {
+		if f.defaultOcpVersion == "" {
+			// No default version available in cache
+			f.baseLog.Error("Cluster creation attempted without --version parameter, but no default OpenShift version is available in cache.")
+			return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "properties.clusterProfile.version", "No default OpenShift version is available. Please specify a version explicitly using the --version parameter.")
+		}
+		oc.Properties.ClusterProfile.Version = f.defaultOcpVersion
+	}
+
+	return nil
+}

pkg/frontend/validate_test.go

@@ -9,6 +9,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/sirupsen/logrus"
+
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	"github.com/Azure/ARO-RP/pkg/api"
@@ -274,34 +276,52 @@ func TestValidateInstallVersion(t *testing.T) {
 	for _, tt := range []struct {
 		test              string
 		version           string
+		defaultOcpVersion string
 		availableVersions []string
 		wantVersion       string
 		wantErr           string
 	}{
 		{
 			test:              "Valid and available OCP version specified returns no error",
 			version:           "4.12.25",
+			defaultOcpVersion: defaultOcpVersion,
 			availableVersions: []string{"4.12.25", "4.13.40", "4.14.16"},
 		},
 		{
 			test:              "No version specified, uses default and returns no error",
+			defaultOcpVersion: defaultOcpVersion,
 			availableVersions: []string{"4.12.25", "4.13.40", "4.14.16"},
 			wantVersion:       "4.12.25",
 		},
+		{
+			test:              "No version specified, defaultOcpVersion empty, no versions available, returns helpful error",
+			defaultOcpVersion: "",
+			availableVersions: []string{},
+			wantErr:           "500: InternalServerError: properties.clusterProfile.version: No default OpenShift version is available. Please specify a version explicitly using the --version parameter.",
+		},
+		{
+			test:              "No version specified, defaultOcpVersion empty, versions available but no default, returns error",
+			defaultOcpVersion: "",
+			availableVersions: []string{"4.12.25", "4.13.40"},
+			wantErr:           "500: InternalServerError: properties.clusterProfile.version: No default OpenShift version is available. Please specify a version explicitly using the --version parameter.",
+		},
 		{
 			test:              "Valid version specified but not available returns error",
 			version:           "4.14.16",
+			defaultOcpVersion: defaultOcpVersion,
 			availableVersions: []string{"4.12.25", "4.13.40"},
 			wantErr:           "400: InvalidParameter: properties.clusterProfile.version: The requested OpenShift version '4.14.16' is invalid.",
 		},
 		{
 			test:              "Prerelease version returns no error",
 			version:           "4.14.0-0.nightly-2024-01-01-000000",
+			defaultOcpVersion: defaultOcpVersion,
 			availableVersions: []string{"4.12.25", "4.13.40", "4.14.16", "4.14.0-0.nightly-2024-01-01-000000"},
 		},
 		{
 			test:              "Version with metadata returns no error",
 			version:           "4.14.16+installerref-abcdef",
+			defaultOcpVersion: defaultOcpVersion,
 			availableVersions: []string{"4.12.25", "4.13.40", "4.14.16", "4.14.16+installerref-abcdef"},
 		},
 	} {
@@ -315,7 +335,8 @@ func TestValidateInstallVersion(t *testing.T) {
 
 			f := frontend{
 				enabledOcpVersions: enabledOcpVersions,
-				defaultOcpVersion:  defaultOcpVersion,
+				defaultOcpVersion:  tt.defaultOcpVersion,
+				baseLog:            logrus.NewEntry(logrus.StandardLogger()),
 			}
 
 			oc := &api.OpenShiftCluster{