Skip to content

Allow PCUI to support multiple Pcluster versions #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Jun 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9865410
Modify cloudformation template and api handler to create multiple api…
hgreebe Apr 21, 2025
f84a92d
Add version field to cluster create
hgreebe Apr 21, 2025
96dfef4
Add dropdown menu to select the image version for the Official Images…
hgreebe Apr 23, 2025
d5a1384
Add a version page to create cluster with a version dropdown menu and…
hgreebe Apr 24, 2025
9461617
Fix location of the version dropdown menu on the official images page
hgreebe Apr 30, 2025
897b3a5
Modify unit tests to account for multiple pcluster versions
hgreebe May 1, 2025
98ed239
Modify demo environment to work with multiple pcluster versions
hgreebe May 1, 2025
ff49a2b
Fix cluster update
hgreebe May 1, 2025
760614a
fix frontend tests
hgreebe May 2, 2025
58eaa88
Use set for version comparison
hgreebe May 2, 2025
7d0ce72
Add version field test
hgreebe May 2, 2025
709b5d6
Add backend test for invoke url mapping
hgreebe May 2, 2025
a1f5c23
Fix official images version dropdown menu
hgreebe May 2, 2025
4199108
Wrap url mapping logic in a function
hgreebe May 5, 2025
40fd88c
Add cluster version selection to custom image build.
hgreebe May 13, 2025
5529745
fix create cluster from template
hgreebe May 19, 2025
6d8dcb8
Scope down permissions
hgreebe Jun 3, 2025
5f5ae6f
Fix formatting
hgreebe Jun 4, 2025
d8169b1
Handle duplicate versions
hgreebe Jun 4, 2025
c9e5652
Fix create version issue
hgreebe Jun 4, 2025
aab0f60
Account for fix of path traversal vulnerability
hgreebe Jun 4, 2025
ac8981a
Merge branch 'main' into develop
hgreebe Jun 4, 2025
52a3ef6
Add images unit tests
hgreebe Jun 8, 2025
2170bb6
Scope down ApiVersionMap permissions
hgreebe Jun 9, 2025
94f9775
Change ui api id tag
hgreebe Jun 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 34 additions & 13 deletions api/PclusterApiHandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are setting the latest PC version as the default one.
The user should be able to control the default version, but with this solution is not.

Example: a user wants to try a new version of PC, but keeping the default version to the one that they consider the most stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because for the home page where all of the clusters are displayed, it uses the list clusters API of the default version.
Currently, if the supported API version is larger than an unsupport version of one of the clusters, then you will be able to see some of its information, there will just be a warning saying that you can not edit the cluster. But is the supported API version it less than the unsupported version, you won't be able to see any cluster information and will getting an error message saying that the version is not supported.

The reason for the sorting/default version being the largest version is so that for example if the supported versions for 3.13.0 and 3.11.0, you will be able to see the information for a 3.12.0 cluster no matter the order you put in the supported versions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the explanation. Please put a comment explaining why the default version must be the most recent one.

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")
Expand All @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand All @@ -191,7 +206,7 @@ def get_scopes_list():

def get_redirect_uri():
return f"{SITE_URL}/login"

# Local Endpoints


Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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)
Expand All @@ -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
18 changes: 17 additions & 1 deletion api/tests/test_pcluster_api_handler.py
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -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,')

24 changes: 23 additions & 1 deletion frontend/locales/en/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.",
Expand Down Expand Up @@ -1407,6 +1419,8 @@
},
"dialogs": {
"buildImage": {
"versionLabel": "Version",
"versionPlaceholder": "Select version",
"closeAriaLabel": "Close",
"title": "Image configuration",
"cancel": "Cancel",
Expand All @@ -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",
Expand Down
12 changes: 10 additions & 2 deletions frontend/src/__tests__/CreateCluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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),
Expand All @@ -55,6 +57,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
clusterConfiguration,
mockRegion,
mockSelectedRegion,
clusterVersion,
false,
mockSuccessCallback,
)
Expand All @@ -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),
Expand Down Expand Up @@ -106,6 +110,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
clusterConfiguration,
mockRegion,
mockSelectedRegion,
clusterVersion,
false,
undefined,
mockErrorCallback,
Expand All @@ -128,6 +133,7 @@ describe('given a CreateCluster command and a cluster configuration', () => {
'Imds:\n ImdsSupport: v2.0',
mockRegion,
mockSelectedRegion,
clusterVersion,
)

expect(mockRequest).toHaveBeenCalledWith(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/__tests__/GetVersion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -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']})
})
})

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/__tests__/ListClusters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions frontend/src/components/__tests__/useLoadingState.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down Expand Up @@ -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',
},
Expand All @@ -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,
},
})
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/feature-flags/useFeatureFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading