Allow PCUI to support multiple Pcluster versions (#418)

* Modify cloudformation template and api handler to create multiple api stack for each version and handle the mapping of version to invoke url

* Add version field to cluster create

* Add dropdown menu to select the image version for the Official Images page

* Add a version page to create cluster with a version dropdown menu and load version when creating cluster from an existing cluster

* Fix location of the version dropdown menu on the official images page

* Modify unit tests to account for multiple pcluster versions

* Modify demo environment to work with multiple pcluster versions

* Fix cluster update

* fix frontend tests

* Use set for version comparison

* Add version field test

* Add backend test for invoke url mapping

* Fix official images version dropdown menu

* Wrap url mapping logic in a function

* Add cluster version selection to custom image build.

* fix create cluster from template

* Scope down permissions

* Fix formatting

* Handle duplicate versions

* Fix create version issue

* Account for fix of path traversal vulnerability

* Add images unit tests

* Scope down ApiVersionMap permissions

* Change ui api id tag

Author: hgreebe

api/PclusterApiHandler.py

@@ -32,13 +32,14 @@
 USER_POOL_ID = os.getenv("USER_POOL_ID")
 AUTH_PATH = os.getenv("AUTH_PATH")
 API_BASE_URL = os.getenv("API_BASE_URL")
-API_VERSION = os.getenv("API_VERSION", "3.1.0")
+API_VERSION = sorted(set(os.getenv("API_VERSION", "3.1.0").strip().split(",")), key=lambda x: [-int(n) for n in x.split('.')])
+# Default version must be highest version so that it can be used for read operations due to backwards compatibility
+DEFAULT_API_VERSION = API_VERSION[0]
 API_USER_ROLE = os.getenv("API_USER_ROLE")
 OIDC_PROVIDER = os.getenv("OIDC_PROVIDER")
 CLIENT_ID = os.getenv("CLIENT_ID")
 CLIENT_SECRET = os.getenv("CLIENT_SECRET")
 SECRET_ID = os.getenv("SECRET_ID")
-SITE_URL = os.getenv("SITE_URL", API_BASE_URL)
 SCOPES_LIST = os.getenv("SCOPES_LIST")
 REGION = os.getenv("AWS_DEFAULT_REGION")
 TOKEN_URL = os.getenv("TOKEN_URL", f"{AUTH_PATH}/oauth2/token")
@@ -48,6 +49,7 @@
 AUDIENCE = os.getenv("AUDIENCE")
 USER_ROLES_CLAIM = os.getenv("USER_ROLES_CLAIM", "cognito:groups")
 SSM_LOG_GROUP_NAME = os.getenv("SSM_LOG_GROUP_NAME")
+ARG_VERSION="version"
 
 try:
     if (not USER_POOL_ID or USER_POOL_ID == "") and SECRET_ID:
@@ -63,6 +65,19 @@
     JWKS_URL = os.getenv("JWKS_URL",
                          f"https://cognito-idp.{REGION}.amazonaws.com/{USER_POOL_ID}/" ".well-known/jwks.json")
 
+def create_url_map(url_list):
+    url_map = {}
+    if url_list:
+        for url in url_list.split(","):
+            if url:
+                pair=url.split("=")
+                url_map[pair[0]] = pair[1]
+    return url_map
+
+API_BASE_URL_MAPPING = create_url_map(API_BASE_URL)
+SITE_URL = os.getenv("SITE_URL", API_BASE_URL_MAPPING.get(DEFAULT_API_VERSION))
+
+
 
 def jwt_decode(token, audience=None, access_token=None):
     return jwt.decode(
@@ -165,7 +180,7 @@ def authenticate(groups):
 
     if (not groups):
         return abort(403)
-        
+
     jwt_roles = set(decoded.get(USER_ROLES_CLAIM, []))
     groups_granted = groups.intersection(jwt_roles)
     if len(groups_granted) == 0:
@@ -191,7 +206,7 @@ def get_scopes_list():
 
 def get_redirect_uri():
   return f"{SITE_URL}/login"
-  
+
 # Local Endpoints
 
 
@@ -233,9 +248,9 @@ def ec2_action():
 def get_cluster_config_text(cluster_name, region=None):
     url = f"/v3/clusters/{cluster_name}"
     if region:
-        info_resp = sigv4_request("GET", API_BASE_URL, url, params={"region": region})
+        info_resp = sigv4_request("GET", get_base_url(request), url, params={"region": region})
     else:
-        info_resp = sigv4_request("GET", API_BASE_URL, url)
+        info_resp = sigv4_request("GET", get_base_url(request), url)
     if info_resp.status_code != 200:
         abort(info_resp.status_code)
 
@@ -365,7 +380,7 @@ def sacct():
             user,
             f"sacct {sacct_args} --json "
             + "| jq -c .jobs[0:120]\\|\\map\\({name,user,partition,state,job_id,exit_code\\}\\)",
-        )
+            )
         if type(accounting) is tuple:
             return accounting
     else:
@@ -484,7 +499,7 @@ def get_dcv_session():
 
 
 def get_custom_image_config():
-    image_info = sigv4_request("GET", API_BASE_URL, f"/v3/images/custom/{request.args.get('image_id')}").json()
+    image_info = sigv4_request("GET", get_base_url(request), f"/v3/images/custom/{request.args.get('image_id')}").json()
     configuration = requests.get(image_info["imageConfiguration"]["url"])
     return configuration.text
 
@@ -596,9 +611,9 @@ def _get_identity_from_token(decoded, claims):
         identity["username"] = decoded["username"]
 
     for claim in claims:
-      if claim in decoded:
-        identity["attributes"][claim] = decoded[claim]
-    
+        if claim in decoded:
+            identity["attributes"][claim] = decoded[claim]
+
     return identity
 
 def get_identity():
@@ -735,14 +750,20 @@ def _get_params(_request):
     params.pop("path")
     return params
 
+def get_base_url(request):
+    version = request.args.get(ARG_VERSION)
+    if version and str(version) in API_VERSION:
+        return API_BASE_URL_MAPPING[str(version)]
+    return API_BASE_URL_MAPPING[DEFAULT_API_VERSION]
+
 
 pc = Blueprint('pc', __name__)
 
 @pc.get('/', strict_slashes=False)
 @authenticated({'admin'})
 @validated(params=PCProxyArgs)
 def pc_proxy_get():
-    response = sigv4_request(request.method, API_BASE_URL, request.args.get("path"), _get_params(request))
+    response = sigv4_request(request.method, get_base_url(request), request.args.get("path"), _get_params(request))
     return response.json(), response.status_code
 
 @pc.route('/', methods=['POST','PUT','PATCH','DELETE'], strict_slashes=False)
@@ -756,5 +777,5 @@ def pc_proxy():
     except:
         pass
 
-    response = sigv4_request(request.method, API_BASE_URL, request.args.get("path"), _get_params(request), body=body)
+    response = sigv4_request(request.method, get_base_url(request), request.args.get("path"), _get_params(request), body=body)
     return response.json(), response.status_code

api/tests/test_pcluster_api_handler.py

@@ -1,5 +1,10 @@
 from unittest import mock
-from api.PclusterApiHandler import login
+from api.PclusterApiHandler import login, get_base_url, create_url_map
+
+class MockRequest:
+    cookies = {'int_value': 100}
+    args = {'version': '3.12.0'}
+    json = {'username': '[email protected]'}
 
 
 @mock.patch("api.PclusterApiHandler.requests.post")
@@ -27,3 +32,14 @@ def test_login_with_no_access_token_returns_401(mocker, app):
         login()
 
         mock_abort.assert_called_once_with(401)
+
+def test_get_base_url(monkeypatch):
+    monkeypatch.setattr('api.PclusterApiHandler.API_VERSION', ['3.12.0', '3.11.0'])
+    monkeypatch.setattr('api.PclusterApiHandler.API_BASE_URL', '3.12.0=https://example.com,3.11.0=https://example1.com,')
+    monkeypatch.setattr('api.PclusterApiHandler.API_BASE_URL_MAPPING', {'3.12.0': 'https://example.com', '3.11.0': 'https://example1.com'})
+
+    assert 'https://example.com' == get_base_url(MockRequest())
+
+def test_create_url_map():
+    assert {'3.12.0': 'https://example.com', '3.11.0': 'https://example1.com'} == create_url_map('3.12.0=https://example.com,3.11.0=https://example1.com,')
+

frontend/locales/en/strings.json

@@ -101,7 +101,7 @@
     "ariaLabel": "Info"
   },
   "cluster": {
-    "editAlert": "This cluster cannot be edited as it was created using a different version of AWS ParallelCluster.",
+    "editAlert": "This cluster cannot be edited as it was created using an incompatible version of AWS ParallelCluster.",
     "tabs": {
       "details": "Details",
       "instances": "Instances",
@@ -493,6 +493,18 @@
       "collapsedStepsLabel": "Step {{stepNumber}} of {{stepsCount}}",
       "steps": "Steps"
     },
+    "version": {
+      "label": "Cluster Version",
+      "title": "Version",
+      "placeholder": "Cluster version",
+      "description": "Select the AWS ParallelCluster version to use for this cluster.",
+      "help": {
+        "main": "Choose the version of AWS ParallelCluster to use for creating and managing your cluster."
+      },
+      "validation": {
+        "versionSelect": "You must select a version."
+      }
+    },
     "cluster": {
       "title": "Cluster",
       "description": "Configure the settings that apply to all cluster resources.",
@@ -1407,6 +1419,8 @@
     },
     "dialogs": {
       "buildImage": {
+        "versionLabel": "Version",
+        "versionPlaceholder": "Select version",
         "closeAriaLabel": "Close",
         "title": "Image configuration",
         "cancel": "Cancel",
@@ -1430,6 +1444,14 @@
         "href": "https://docs.aws.amazon.com/parallelcluster/latest/ug/support-policy.html"
       }
     },
+    "actions": {
+      "versionSelect": {
+        "selectedAriaLabel": "Selected version",
+        "versionText": "Version {{version}}",
+        "placeholder": "Select a version"
+      },
+      "refresh": "Refresh"
+    },
     "list": {
       "columns": {
         "id": "ID",

frontend/src/__tests__/CreateCluster.test.ts

@@ -8,6 +8,7 @@ const mockRequest = executeRequest as jest.Mock
 
 describe('given a CreateCluster command and a cluster configuration', () => {
   const clusterName = 'any-name'
+  const clusterVersion = 'some-version'
   const clusterConfiguration = 'Imds:\n  ImdsSupport: v2.0'
   const mockRegion = 'some-region'
   const mockSelectedRegion = 'some-region'
@@ -37,11 +38,12 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         clusterConfiguration,
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
       )
       expect(mockRequest).toHaveBeenCalledTimes(1)
       expect(mockRequest).toHaveBeenCalledWith(
         'post',
-        'api?path=/v3/clusters&region=some-region',
+        'api?path=/v3/clusters&region=some-region&version=some-version',
         expectedBody,
         expect.any(Object),
         expect.any(Object),
@@ -55,6 +57,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         clusterConfiguration,
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
         false,
         mockSuccessCallback,
       )
@@ -71,12 +74,13 @@ describe('given a CreateCluster command and a cluster configuration', () => {
           clusterConfiguration,
           mockRegion,
           mockSelectedRegion,
+          clusterVersion,
           mockDryRun,
         )
         expect(mockRequest).toHaveBeenCalledTimes(1)
         expect(mockRequest).toHaveBeenCalledWith(
           'post',
-          'api?path=/v3/clusters&dryrun=true&region=some-region',
+          'api?path=/v3/clusters&dryrun=true&region=some-region&version=some-version',
           expect.any(Object),
           expect.any(Object),
           expect.any(Object),
@@ -106,6 +110,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         clusterConfiguration,
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
         false,
         undefined,
         mockErrorCallback,
@@ -128,6 +133,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         'Imds:\n  ImdsSupport: v2.0',
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
       )
 
       expect(mockRequest).toHaveBeenCalledWith(
@@ -154,6 +160,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         'Imds:\n  ImdsSupport: v2.0\nTags:\n  - Key: foo\n    Value: bar',
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
       )
 
       expect(mockRequest).toHaveBeenCalledWith(
@@ -180,6 +187,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
         "Imds:\n  ImdsSupport: v2.0\nTags:\n  - Key: parallelcluster-ui\n    Value: 'true'",
         mockRegion,
         mockSelectedRegion,
+        clusterVersion,
       )
 
       expect(mockRequest).not.toHaveBeenCalledWith(

frontend/src/__tests__/GetVersion.test.ts

@@ -30,7 +30,7 @@ describe('given a GetVersion command', () => {
   describe('when the PC version can be retrieved successfully', () => {
     beforeEach(() => {
       const mockResponse = {
-        version: '3.5.0',
+        version: ['3.5.0', '3.6.0'],
       }
 
       mockGet.mockResolvedValueOnce({data: mockResponse})
@@ -39,13 +39,13 @@ describe('given a GetVersion command', () => {
     it('should return the PC version', async () => {
       const data = await GetVersion()
 
-      expect(data).toEqual<PCVersion>({full: '3.5.0'})
+      expect(data).toEqual<PCVersion>({full: ['3.5.0', '3.6.0']})
     })
 
     it('should store the PC version', async () => {
       await GetVersion()
 
-      expect(setState).toHaveBeenCalledWith(['app', 'version'], {full: '3.5.0'})
+      expect(setState).toHaveBeenCalledWith(['app', 'version'], {full: ['3.5.0', '3.6.0']})
     })
   })
 

frontend/src/__tests__/ListClusters.test.ts

@@ -14,7 +14,7 @@ const mockCluster1: ClusterInfoSummary = {
 const mockCluster2: ClusterInfoSummary = {
   clusterName: 'test-cluster-2',
   clusterStatus: ClusterStatus.CreateComplete,
-  version: '3.8.0',
+  version: '3.9.0',
   cloudformationStackArn: 'arn',
   region: 'region',
   cloudformationStackStatus: CloudFormationStackStatus.CreateComplete,

frontend/src/components/__tests__/useLoadingState.test.tsx

@@ -76,7 +76,7 @@ describe('given a hook to load all the data necessary for the app to boot', () =
           someKey: 'some-value',
         },
         app: {
-          version: {full: '3.5.0'},
+          version: {full: ['3.5.0', '3.7.0']},
           appConfig: {
             someKey: 'some-value',
           },
@@ -109,7 +109,7 @@ describe('given a hook to load all the data necessary for the app to boot', () =
       mockStore.getState.mockReturnValue({
         identity: null,
         app: {
-          version: {full: '3.5.0'},
+          wizard: {version: '3.5.0'},
           appConfig: {
             someKey: 'some-value',
           },
@@ -131,7 +131,9 @@ describe('given a hook to load all the data necessary for the app to boot', () =
           someKey: 'some-value',
         },
         app: {
-          version: {full: '3.5.0'},
+          wizard: {
+            version: '3.5.0',
+          },
           appConfig: null,
         },
       })

frontend/src/feature-flags/useFeatureFlag.ts

@@ -13,7 +13,7 @@ import {featureFlagsProvider} from './featureFlagsProvider'
 import {AvailableFeature} from './types'
 
 export function useFeatureFlag(feature: AvailableFeature): boolean {
-  const version = useState(['app', 'version', 'full'])
+  const version = useState(['app', 'wizard', 'version'])
   const region = useState(['aws', 'region'])
   return isFeatureEnabled(version, region, feature)
 }