Installer images use sha256 (Azure#2967)

* Fix equality logic on version * Leverage image digests when using the aro-installer image * frontend: Discontinue adding default to enabled OCP versions The CosmosDB "OpenShiftVersions" container in all environments is now primed with supported OCP versions. RP no longer needs to hard code a default in enabledOcpVersions. * Insert the default version into cosmosdb on cluster creation in localdevmode --------- Co-authored-by: Matthew Barnes <[email protected]>
bennerv · Jul 3, 2023 · 0378d0e · 0378d0e
1 parent 3d758d2
commit 0378d0e
Show file tree

Hide file tree

Showing 12 changed files with 145 additions and 87 deletions.
diff --git a/cmd/aro/update_ocp_versions.go b/cmd/aro/update_ocp_versions.go
@@ -31,11 +31,18 @@ func getLatestOCPVersions(ctx context.Context, log *logrus.Entry) ([]api.OpenShi
 	ocpVersions := []api.OpenShiftVersion{}
 
 	for _, vers := range version.HiveInstallStreams {
+		installerPullSpec := fmt.Sprintf("%s/aro-installer:%s", dstRepo, vers.Version.MinorVersion())
+		digest, ok := version.InstallerImageDigest[vers.Version.MinorVersion()]
+		if !ok {
+			return nil, fmt.Errorf("no digest found for version %s", vers.Version.String())
+		}
+
+		installerPullSpec = fmt.Sprintf("%s@sha256:%s", installerPullSpec, digest)
 		ocpVersions = append(ocpVersions, api.OpenShiftVersion{
 			Properties: api.OpenShiftVersionProperties{
 				Version:           vers.Version.String(),
 				OpenShiftPullspec: vers.PullSpec,
-				InstallerPullspec: fmt.Sprintf("%s/aro-installer:release-%s", dstRepo, vers.Version.MinorVersion()),
+				InstallerPullspec: installerPullSpec,
 				Enabled:           true,
 			},
 		})

diff --git a/pkg/cluster/install_version_test.go b/pkg/cluster/install_version_test.go
@@ -5,7 +5,6 @@ package cluster
 
 import (
 	"context"
-	"fmt"
 	"strings"
 	"testing"
 
@@ -14,7 +13,6 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
-	"github.com/Azure/ARO-RP/pkg/util/version"
 	testdatabase "github.com/Azure/ARO-RP/test/database"
 	"github.com/Azure/ARO-RP/test/util/deterministicuuid"
 	"github.com/Azure/ARO-RP/test/util/testliveconfig"
@@ -32,32 +30,6 @@ func TestGetOpenShiftVersionFromVersion(t *testing.T) {
 		wantErrString string
 		want          *api.OpenShiftVersion
 	}{
-		{
-			name: "no versions gets default version",
-			f:    func(f *testdatabase.Fixture) {},
-			m: manager{
-				doc: &api.OpenShiftClusterDocument{
-					Key: strings.ToLower(key),
-					OpenShiftCluster: &api.OpenShiftCluster{
-						ID: key,
-						Properties: api.OpenShiftClusterProperties{
-							ClusterProfile: api.ClusterProfile{
-								Version: version.DefaultInstallStream.Version.String(),
-							},
-						},
-					},
-				},
-				openShiftClusterDocumentVersioner: new(openShiftClusterDocumentVersionerService),
-			},
-			wantErrString: "",
-			want: &api.OpenShiftVersion{
-				Properties: api.OpenShiftVersionProperties{
-					Version:           version.DefaultInstallStream.Version.String(),
-					OpenShiftPullspec: version.DefaultInstallStream.PullSpec,
-					InstallerPullspec: fmt.Sprintf("%s/aro-installer:release-%d.%d", testACRDomain, version.DefaultInstallStream.Version.V[0], version.DefaultInstallStream.Version.V[1]),
-				},
-			},
-		},
 		{
 			name: "select nonexistent version",
 			f: func(f *testdatabase.Fixture) {

diff --git a/pkg/cluster/version.go b/pkg/cluster/version.go
@@ -5,14 +5,12 @@ package cluster
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"strings"
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/database"
 	"github.com/Azure/ARO-RP/pkg/env"
-	"github.com/Azure/ARO-RP/pkg/util/version"
 )
 
 // openShiftClusterDocumentVersioner is the interface that validates and obtains the version from an OpenShiftClusterDocument.
@@ -28,28 +26,6 @@ type openShiftClusterDocumentVersionerService struct{}
 func (service *openShiftClusterDocumentVersionerService) Get(ctx context.Context, doc *api.OpenShiftClusterDocument, dbOpenShiftVersions database.OpenShiftVersions, env env.Interface, installViaHive bool) (*api.OpenShiftVersion, error) {
 	requestedInstallVersion := doc.OpenShiftCluster.Properties.ClusterProfile.Version
 
-	// Honor any installer pull spec override
-	installerPullSpec := env.LiveConfig().DefaultInstallerPullSpecOverride(ctx)
-	if installerPullSpec == "" {
-		installerPullSpec = fmt.Sprintf("%s/aro-installer:release-%s", env.ACRDomain(), version.DefaultInstallStream.Version.MinorVersion())
-	}
-
-	// add the default OCP version as we require it as an install target
-	// if this is removed, we need to also update the logic in
-	// pkg/frontend/frontend.go, pkg/frontend/validate.go
-	defaultVersion := &api.OpenShiftVersion{
-		Properties: api.OpenShiftVersionProperties{
-			Version:           version.DefaultInstallStream.Version.String(),
-			OpenShiftPullspec: version.DefaultInstallStream.PullSpec,
-			InstallerPullspec: installerPullSpec,
-			Enabled:           true,
-		},
-	}
-
-	if requestedInstallVersion == defaultVersion.Properties.Version {
-		return defaultVersion, nil
-	}
-
 	// TODO: Refactor to use changefeeds rather than querying the database every time
 	// should also leverage shared changefeed or shared logic
 	docs, err := dbOpenShiftVersions.ListAll(ctx)
@@ -66,31 +42,17 @@ func (service *openShiftClusterDocumentVersionerService) Get(ctx context.Context
 
 	errUnsupportedVersion := api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.clusterProfile.version", "The requested OpenShift version '%s' is not supported.", requestedInstallVersion)
 
-	// when we have no OpenShiftVersion entries in CosmoDB, default to building one using the DefaultInstallStream
-	if len(activeOpenShiftVersions) == 0 {
-		if requestedInstallVersion != version.DefaultInstallStream.Version.String() {
-			return nil, errUnsupportedVersion
-		}
-
-		openshiftPullSpec := version.DefaultInstallStream.PullSpec
-		if installViaHive {
-			openshiftPullSpec = strings.Replace(openshiftPullSpec, "quay.io", env.ACRDomain(), 1)
-		}
-
-		return &api.OpenShiftVersion{
-			Properties: api.OpenShiftVersionProperties{
-				Version:           version.DefaultInstallStream.Version.String(),
-				OpenShiftPullspec: openshiftPullSpec,
-				InstallerPullspec: installerPullSpec,
-				Enabled:           true,
-			}}, nil
-	}
-
 	for _, active := range activeOpenShiftVersions {
 		if requestedInstallVersion == active.Properties.Version {
 			if installViaHive {
 				active.Properties.OpenShiftPullspec = strings.Replace(active.Properties.OpenShiftPullspec, "quay.io", env.ACRDomain(), 1)
 			}
+
+			// Honor any pull spec override set
+			installerPullSpecOverride := env.LiveConfig().DefaultInstallerPullSpecOverride(ctx)
+			if installerPullSpecOverride != "" {
+				active.Properties.InstallerPullspec = installerPullSpecOverride
+			}
 			return active, nil
 		}
 	}

diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go
@@ -33,7 +33,6 @@ import (
 	"github.com/Azure/ARO-RP/pkg/util/heartbeat"
 	utillog "github.com/Azure/ARO-RP/pkg/util/log"
 	"github.com/Azure/ARO-RP/pkg/util/recover"
-	"github.com/Azure/ARO-RP/pkg/util/version"
 )
 
 type statusCodeError int
@@ -164,15 +163,7 @@ func NewFrontend(ctx context.Context,
 
 		clusterEnricher: enricher,
 
-		// add default installation version so it's always supported
-		enabledOcpVersions: map[string]*api.OpenShiftVersion{
-			version.DefaultInstallStream.Version.String(): {
-				Properties: api.OpenShiftVersionProperties{
-					Version: version.DefaultInstallStream.Version.String(),
-					Enabled: true,
-				},
-			},
-		},
+		enabledOcpVersions: map[string]*api.OpenShiftVersion{},
 
 		bucketAllocator: &bucket.Random{},
 

diff --git a/pkg/frontend/openshiftcluster_preflightvalidation_test.go b/pkg/frontend/openshiftcluster_preflightvalidation_test.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/metrics/noop"
+	"github.com/Azure/ARO-RP/pkg/util/version"
 	testdatabase "github.com/Azure/ARO-RP/test/database"
 )
 
@@ -112,13 +113,22 @@ func TestPreflightValidation(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			f, err := NewFrontend(ctx, ti.audit, ti.log, ti.env, ti.asyncOperationsDatabase, ti.clusterManagerDatabase, ti.openShiftClustersDatabase, ti.subscriptionsDatabase, nil, api.APIs, &noop.Noop{}, nil, nil, nil, nil, nil)
+			f, err := NewFrontend(ctx, ti.audit, ti.log, ti.env, ti.asyncOperationsDatabase, ti.clusterManagerDatabase, ti.openShiftClustersDatabase, ti.subscriptionsDatabase, ti.openShiftVersionsDatabase, api.APIs, &noop.Noop{}, nil, nil, nil, nil, nil)
 			if err != nil {
 				t.Fatal(err)
 			}
 			oc := tt.preflightRequest()
 
 			go f.Run(ctx, nil, nil)
+			f.mu.Lock()
+			f.enabledOcpVersions = map[string]*api.OpenShiftVersion{
+				version.DefaultInstallStream.Version.String(): {
+					Properties: api.OpenShiftVersionProperties{
+						Version: version.DefaultInstallStream.Version.String(),
+					},
+				},
+			}
+			f.mu.Unlock()
 
 			headers := http.Header{
 				"Content-Type": []string{"application/json"},

diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go
@@ -649,6 +649,15 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 		},
 	}
 
+	defaultVersionChangeFeed := map[string]*api.OpenShiftVersion{
+		version.DefaultInstallStream.Version.String(): {
+			Properties: api.OpenShiftVersionProperties{
+				Version: version.DefaultInstallStream.Version.String(),
+				Enabled: true,
+			},
+		},
+	}
+
 	mockSubID := "00000000-0000-0000-0000-000000000000"
 	mockCurrentTime := time.Date(1970, 1, 1, 0, 0, 0, 0, time.UTC)
 
@@ -657,6 +666,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 		request                 func(*v20200430.OpenShiftCluster)
 		isPatch                 bool
 		fixture                 func(*testdatabase.Fixture)
+		changeFeed              map[string]*api.OpenShiftVersion
 		quotaValidatorError     error
 		skuValidatorError       error
 		providersValidatorError error
@@ -686,6 +696,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:             defaultVersionChangeFeed,
 			wantSystemDataEnriched: true,
 			wantDocuments: func(c *testdatabase.Checker) {
 				c.AddAsyncOperationDocuments(&api.AsyncOperationDocument{
@@ -758,6 +769,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:          defaultVersionChangeFeed,
 			quotaValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The provided VM SKU %s is not supported.", "something"),
 			wantEnriched:        []string{},
 			wantStatusCode:      http.StatusBadRequest,
@@ -779,6 +791,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:          defaultVersionChangeFeed,
 			quotaValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeQuotaExceeded, "", "Resource quota of vm exceeded. Maximum allowed: 0, Current in use: 0, Additional requested: 1."),
 			wantEnriched:        []string{},
 			wantStatusCode:      http.StatusBadRequest,
@@ -800,6 +813,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:        defaultVersionChangeFeed,
 			skuValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The selected SKU '%v' is unavailable in region '%v'", "Standard_Sku", "somewhere"),
 			wantEnriched:      []string{},
 			wantStatusCode:    http.StatusBadRequest,
@@ -821,6 +835,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:        defaultVersionChangeFeed,
 			skuValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The selected SKU '%v' is restricted in region '%v' for selected subscription", "Standard_Sku", "somewhere"),
 			wantEnriched:      []string{},
 			wantStatusCode:    http.StatusBadRequest,
@@ -843,6 +858,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:              defaultVersionChangeFeed,
 			providersValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeResourceProviderNotRegistered, "", "The resource provider '%s' is not registered.", "Microsoft.Authorization"),
 			wantEnriched:            []string{},
 			wantStatusCode:          http.StatusBadRequest,
@@ -864,6 +880,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:              defaultVersionChangeFeed,
 			providersValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeResourceProviderNotRegistered, "", "The resource provider '%s' is not registered.", "Microsoft.Compute"),
 			wantEnriched:            []string{},
 			wantStatusCode:          http.StatusBadRequest,
@@ -885,6 +902,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:              defaultVersionChangeFeed,
 			providersValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeResourceProviderNotRegistered, "", "The resource provider '%s' is not registered.", "Microsoft.Network"),
 			wantEnriched:            []string{},
 			wantStatusCode:          http.StatusBadRequest,
@@ -906,6 +924,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:              defaultVersionChangeFeed,
 			providersValidatorError: api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeResourceProviderNotRegistered, "", "The resource provider '%s' is not registered.", "Microsoft.Storage"),
 			wantEnriched:            []string{},
 			wantStatusCode:          http.StatusBadRequest,
@@ -1515,6 +1534,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:             defaultVersionChangeFeed,
 			wantSystemDataEnriched: true,
 			wantAsync:              true,
 			wantStatusCode:         http.StatusBadRequest,
@@ -1560,6 +1580,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 					},
 				})
 			},
+			changeFeed:             defaultVersionChangeFeed,
 			wantSystemDataEnriched: true,
 			wantAsync:              true,
 			wantStatusCode:         http.StatusBadRequest,
@@ -1608,6 +1629,9 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) {
 			}
 
 			go f.Run(ctx, nil, nil)
+			f.mu.Lock()
+			f.enabledOcpVersions = tt.changeFeed
+			f.mu.Unlock()
 
 			oc := &v20200430.OpenShiftCluster{}
 			if tt.request != nil {

diff --git a/pkg/frontend/validate.go b/pkg/frontend/validate.go
@@ -205,13 +205,12 @@ func validateAdminMasterVMSize(vmSize string) error {
 func (f *frontend) validateInstallVersion(ctx context.Context, doc *api.OpenShiftCluster) error {
 	// If this request is from an older API or the user never specified
 	// the version to install we default to the DefaultInstallStream.Version
+	// TODO: We should set default version in cosmosdb instead of hardcoding it in golang code
 	if doc.Properties.ClusterProfile.Version == "" {
 		doc.Properties.ClusterProfile.Version = version.DefaultInstallStream.Version.String()
-		return nil
 	}
 
 	f.mu.RLock()
-	// we add the default installation version to the enabled ocp versions so no special case
 	_, ok := f.enabledOcpVersions[doc.Properties.ClusterProfile.Version]
 	f.mu.RUnlock()