Skip to content

Commit e415089

Browse files
PreetiWPreetiW
committed
chore: cleanup for checkPermission.ts and fixes error in /access endpoint
1 parent 4e39fb0 commit e415089

File tree

6 files changed

+80
-174
lines changed

6 files changed

+80
-174
lines changed

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-backend/src/routes/access.ts‎

Lines changed: 46 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@ import {
2323
import { rosPluginPermissions } from '@red-hat-developer-hub/plugin-redhat-resource-optimization-common/permissions';
2424
import { getTokenFromApi } from '../util/tokenUtil';
2525
import { AuthorizeResult } from '@backstage/plugin-permission-common';
26-
import { deepMapKeys } from '@red-hat-developer-hub/plugin-redhat-resource-optimization-common/json-utils';
27-
import camelCase from 'lodash/camelCase';
28-
import { RecommendationList } from '@red-hat-developer-hub/plugin-redhat-resource-optimization-common';
2926

3027
export const getAccess: (options: RouterOptions) => RequestHandler =
3128
options => async (_, response) => {
@@ -72,39 +69,51 @@ export const getAccess: (options: RouterOptions) => RequestHandler =
7269
clusterDataMap = clusterMapDataFromCache;
7370
allProjects = projectDataFromCache;
7471
} else {
75-
// token
76-
const token = await getTokenFromApi(options);
77-
78-
// hit /recommendation API endpoint
79-
const optimizationResponse = await optimizationApi.getRecommendationList(
80-
{
81-
query: {
82-
limit: -1,
83-
orderHow: 'desc',
84-
orderBy: 'last_reported',
85-
},
86-
},
87-
{ token },
88-
);
89-
90-
if (optimizationResponse.ok) {
91-
const data = await optimizationResponse.json();
92-
const camelCaseTransformedResponse = deepMapKeys(
93-
data,
94-
camelCase as (value: string | number) => string,
95-
) as RecommendationList;
72+
try {
73+
// token
74+
const token = await getTokenFromApi(options);
75+
76+
// hit /recommendation API endpoint
77+
const optimizationResponse =
78+
await optimizationApi.getRecommendationList(
79+
{
80+
query: {
81+
limit: -1,
82+
orderHow: 'desc',
83+
orderBy: 'last_reported',
84+
},
85+
},
86+
{ token },
87+
);
88+
89+
// OptimizationsClient already transforms to camelCase when token is provided
90+
const recommendationList = await optimizationResponse.json();
91+
92+
// Check if response contains errors
93+
if ((recommendationList as any).errors) {
94+
logger.error(
95+
'API returned errors:',
96+
(recommendationList as any).errors,
97+
);
98+
return response.status(500).json({
99+
decision: AuthorizeResult.DENY,
100+
error: 'API returned errors',
101+
authorizeClusterIds: [],
102+
authorizeProjects: [],
103+
});
104+
}
96105

97106
// retrive cluster data from the API result
98-
if (camelCaseTransformedResponse.data) {
99-
camelCaseTransformedResponse.data.map(recommendation => {
107+
if (recommendationList.data) {
108+
recommendationList.data.map(recommendation => {
100109
if (recommendation.clusterAlias && recommendation.clusterUuid)
101110
clusterDataMap[recommendation.clusterAlias] =
102111
recommendation.clusterUuid;
103112
});
104113

105114
allProjects = [
106115
...new Set(
107-
camelCaseTransformedResponse.data.map(
116+
recommendationList.data.map(
108117
recommendation => recommendation.project,
109118
),
110119
),
@@ -118,19 +127,16 @@ export const getAccess: (options: RouterOptions) => RequestHandler =
118127
ttl: 15 * 60 * 1000,
119128
});
120129
}
121-
} else {
122-
const errorText = await optimizationResponse
123-
.text()
124-
.catch(() => 'No error details');
125-
logger.error(
126-
`Failed to fetch recommendations: ${optimizationResponse.status} ${optimizationResponse.statusText}`,
127-
{ errorDetails: errorText },
128-
);
129-
throw new Error(
130-
`Failed to fetch recommendations: ${optimizationResponse.status} ${
131-
optimizationResponse.statusText || 'Unknown error'
132-
}`,
133-
);
130+
} catch (error) {
131+
logger.error('Error fetching recommendations', error);
132+
133+
// Return unauthorized response on any error
134+
return response.status(500).json({
135+
decision: AuthorizeResult.DENY,
136+
error: 'Failed to fetch cluster data',
137+
authorizeClusterIds: [],
138+
authorizeProjects: [],
139+
});
134140
}
135141
}
136142

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-backend/src/routes/costManagementAccess.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,15 @@ export const getCostManagementAccess: (
123123
}),
124124
]);
125125
} catch (error) {
126-
logger.error(`Failed to fetch data from Cost Management API`, error);
127-
throw error;
126+
logger.error('Error fetching cost management data', error);
127+
128+
// Return unauthorized response on any error
129+
return response.status(500).json({
130+
decision: AuthorizeResult.DENY,
131+
error: 'Failed to fetch cluster data',
132+
authorizedClusterNames: [],
133+
authorizeProjects: [],
134+
});
128135
}
129136
}
130137

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-backend/src/service/optimizationsService.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
} from '@backstage/backend-plugin-api';
2222
import type { OptimizationsApi } from '@red-hat-developer-hub/plugin-redhat-resource-optimization-common/clients';
2323
import { OptimizationsClient } from '@red-hat-developer-hub/plugin-redhat-resource-optimization-common/clients';
24-
import { DEFAULT_API_BASE_URL } from '../util/constant';
24+
import { DEFAULT_COST_MANAGEMENT_PROXY_BASE_URL } from '../util/constant';
2525

2626
export const optimizationServiceRef = createServiceRef<OptimizationsApi>({
2727
id: 'optimization-client',
@@ -32,9 +32,10 @@ export const optimizationServiceRef = createServiceRef<OptimizationsApi>({
3232
configApi: coreServices.rootConfig,
3333
},
3434
async factory({ configApi }): Promise<OptimizationsApi> {
35+
// Base URL without /cost-management/v1 since OptimizationsClient appends it
3536
const baseUrl =
3637
configApi.getOptionalString('optimizationsBaseUrl') ??
37-
DEFAULT_API_BASE_URL;
38+
DEFAULT_COST_MANAGEMENT_PROXY_BASE_URL;
3839

3940
return new OptimizationsClient({
4041
discoveryApi: {

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-backend/src/util/checkPermissions.ts‎

Lines changed: 0 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -82,134 +82,6 @@ export const authorize = async (
8282
);
8383
};
8484

85-
/**
86-
* Filters cluster IDs based on cluster-specific permissions.
87-
* @param request - The HTTP request
88-
* @param permissionsSvc - The permissions service
89-
* @param httpAuth - The HTTP auth service
90-
* @param clusterDataMap - Map of clusterName → clusterId
91-
* @param permissionType - 'ros' for ros.{clusterName} or 'cost' for cost.{clusterName} (defaults to 'ros')
92-
* @returns Array of authorized cluster IDs
93-
*/
94-
export const filterAuthorizedClusterIds = async (
95-
request: HttpRequest,
96-
permissionsSvc: PermissionsService,
97-
httpAuth: HttpAuthService,
98-
clusterDataMap: Record<string, string>,
99-
permissionType: ClusterPermissionType = 'ros',
100-
): Promise<string[]> => {
101-
const credentials = await httpAuth.credentials(request);
102-
const allClusterNames: string[] = Object.keys(clusterDataMap);
103-
104-
// Select the appropriate permission function based on type
105-
const getClusterPermission =
106-
permissionType === 'cost'
107-
? costClusterSpecificPermission
108-
: rosClusterSpecificPermission;
109-
110-
const specificClusterRequests: AuthorizePermissionRequest[] =
111-
allClusterNames.map(clusterName => ({
112-
permission: getClusterPermission(clusterName),
113-
}));
114-
115-
const decisions = await permissionsSvc.authorize(specificClusterRequests, {
116-
credentials,
117-
});
118-
119-
const authorizeClusterNames = allClusterNames.filter(
120-
(_, idx) => decisions[idx].result === AuthorizeResult.ALLOW,
121-
);
122-
123-
const authorizedClusterIds = authorizeClusterNames.map(
124-
clusterName => clusterDataMap[clusterName],
125-
);
126-
127-
return permissionType === 'cost'
128-
? authorizeClusterNames
129-
: authorizedClusterIds;
130-
};
131-
132-
/**
133-
* Filters cluster-project combinations based on cluster+project-specific permissions.
134-
* @param request - The HTTP request
135-
* @param permissionsSvc - The permissions service
136-
* @param httpAuth - The HTTP auth service
137-
* @param clusterDataMap - Map of clusterName → clusterId
138-
* @param allProjects - Array of all project names
139-
* @param permissionType - 'ros' for ros.{clusterName}.{projectName} or 'cost' for cost.{clusterName}.{projectName} (defaults to 'ros')
140-
* @returns Array of authorized cluster-project combinations
141-
*/
142-
export const filterAuthorizedClusterProjectIds = async (
143-
request: HttpRequest,
144-
permissionsSvc: PermissionsService,
145-
httpAuth: HttpAuthService,
146-
clusterDataMap: Record<string, string>,
147-
allProjects: string[],
148-
permissionType: ClusterPermissionType = 'ros',
149-
): Promise<ClusterProjectResult[]> => {
150-
const credentials = await httpAuth.credentials(request);
151-
const allClusterNames: string[] = Object.keys(clusterDataMap);
152-
const allClusterIds: string[] = Object.values(clusterDataMap);
153-
154-
// Pre-calculate total combinations for better performance
155-
const totalCombinations = allClusterNames.length * allProjects.length;
156-
157-
// Early exit if no combinations to check
158-
if (totalCombinations === 0) {
159-
return [];
160-
}
161-
162-
// Select the appropriate permission function based on type
163-
const getClusterProjectPermission =
164-
permissionType === 'cost'
165-
? costClusterProjectPermission
166-
: rosClusterProjectPermission;
167-
168-
// Pre-allocate arrays with known size for better memory performance
169-
const specificClusterProjectRequests: AuthorizePermissionRequest[] =
170-
new Array(totalCombinations);
171-
const clusterProjectMap: ClusterProjectResult[] = new Array(
172-
totalCombinations,
173-
);
174-
175-
// Build permission requests and mapping in a single pass
176-
let idx = 0;
177-
for (let i = 0; i < allClusterNames.length; i++) {
178-
const clusterName = allClusterNames[i];
179-
const clusterId = allClusterIds[i];
180-
181-
for (let j = 0; j < allProjects.length; j++) {
182-
const projectName = allProjects[j];
183-
184-
specificClusterProjectRequests[idx] = {
185-
permission: getClusterProjectPermission(clusterName, projectName),
186-
};
187-
188-
clusterProjectMap[idx] = {
189-
cluster: clusterId,
190-
project: projectName,
191-
};
192-
193-
idx++;
194-
}
195-
}
196-
197-
// Batch authorize all permissions in a single call
198-
const decisions = await permissionsSvc.authorize(
199-
specificClusterProjectRequests,
200-
{
201-
credentials,
202-
},
203-
);
204-
205-
// Filter authorized combinations
206-
const finalResult = clusterProjectMap.filter(
207-
(_, index) => decisions[index].result === AuthorizeResult.ALLOW,
208-
);
209-
210-
return finalResult;
211-
};
212-
21385
/**
21486
* Combines cluster-only and cluster-project permission checks into a single optimized flow.
21587
* Uses permission hierarchy where project-level access also grants cluster access.

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-backend/src/util/constant.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616

1717
export const DEFAULT_SSO_BASE_URL = 'https://sso.redhat.com';
18-
export const DEFAULT_API_BASE_URL =
19-
'https://console.redhat.com/api/cost-management/v1';
2018

2119
// Base URL without the /cost-management/v1 path since the client appends it
2220
export const DEFAULT_COST_MANAGEMENT_PROXY_BASE_URL =

‎workspaces/redhat-resource-optimization/plugins/redhat-resource-optimization-common/src/clients/optimizations/OptimizationsClient.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,34 @@ export class OptimizationsClient implements OptimizationsApi {
118118

119119
public async getRecommendationList(
120120
request: GetRecommendationListRequest,
121+
options?: RequestOptions,
121122
): Promise<TypedResponse<RecommendationList>> {
122123
const snakeCaseTransformedRequest = deepMapKeys(
123124
request,
124125
snakeCase as (value: string | number) => string,
125126
) as GetRecommendationListRequest;
126127

128+
// If token is provided in options (backend use case), skip access check
129+
if (options?.token) {
130+
const response = await this.defaultClient.getRecommendationList(
131+
snakeCaseTransformedRequest,
132+
options,
133+
);
134+
135+
return {
136+
...response,
137+
json: async () => {
138+
const data = await response.json();
139+
const camelCaseTransformedResponse = deepMapKeys(
140+
data,
141+
camelCase as (value: string | number) => string,
142+
) as RecommendationList;
143+
return camelCaseTransformedResponse;
144+
},
145+
};
146+
}
147+
148+
// Frontend use case - use fetchWithToken which includes access check
127149
const response = await this.fetchWithToken(
128150
this.defaultClient.getRecommendationList,
129151
snakeCaseTransformedRequest,

0 commit comments

Comments
 (0)