Skip to content
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

fix: prevent redundant looping #600

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ async function configure(context) {

const [environmentScService] = await context.service(['environmentScService']);
const result = await environmentScService.mustFind(requestContext, { id });
await environmentScService.verifyAppStreamConfig(requestContext, result.projectId);
res.status(200).json(result);
}),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class EnvironmentScCidrService extends Service {
);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await environmentScService.verifyAppStreamConfig(requestContext, id);
await environmentScService.verifyAppStreamConfig(requestContext, existingEnvironment.projectId);

await lockService.tryWriteLockAndRun({ id: `${id}-CidrUpdate` }, async () => {
// Calculate diff and update CIDR ranges in ingress rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ class EnvironmentScConnectionService extends Service {
}

// Verify environment is linked to an AppStream project when application has AppStream enabled
await environmentScService.verifyAppStreamConfig(requestContext, envId);
const { projectId } = await environmentScService.mustFind(requestContext, { id: envId });
await environmentScService.verifyAppStreamConfig(requestContext, projectId);

if (_.toLower(_.get(connection, 'type', '')) === 'sagemaker') {
const sagemaker = await environmentScService.getClientSdkWithEnvMgmtRole(
Expand Down Expand Up @@ -241,7 +242,8 @@ class EnvironmentScConnectionService extends Service {
await validationService.ensureValid(sshConnectionInfo, sshConnectionInfoSchema);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await environmentScService.verifyAppStreamConfig(requestContext, envId);
const { projectId } = await environmentScService.mustFind(requestContext, { id: envId });
await environmentScService.verifyAppStreamConfig(requestContext, projectId);

// The following will succeed only if the user has permissions to access the specified environment
const connection = await this.mustFindConnection(requestContext, envId, connectionId);
Expand Down Expand Up @@ -313,7 +315,8 @@ class EnvironmentScConnectionService extends Service {
]);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await environmentScService.verifyAppStreamConfig(requestContext, envId);
const { projectId } = await environmentScService.mustFind(requestContext, { id: envId });
await environmentScService.verifyAppStreamConfig(requestContext, projectId);

// The following will succeed only if the user has permissions to access the specified environment
// and connection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,6 @@ class EnvironmentScService extends Service {
await this.assertAuthorized(requestContext, { action: 'get-sc', conditions: [this._allowAuthorized] }, result);
}

// Verify environment is linked to an AppStream project when application has AppStream enabled
await this.verifyAppStreamConfig(requestContext, id);

const env = this._fromDbToDataObject(result);

// We only check for the ingress rules of a successfully provisioned environment not in failure state
Expand Down Expand Up @@ -470,10 +467,9 @@ class EnvironmentScService extends Service {
return dbResult;
}

async verifyAppStreamConfig(requestContext, envId) {
async verifyAppStreamConfig(requestContext, projectId) {
// If the AppStream feature is enabled, verify the project linked to the environment has it configured
if (this.isAppStreamEnabled) {
const { projectId } = await this.mustFind(requestContext, { id: envId });
const projectService = await this.service('projectService');
// The isAppStreamConfigured attribute value will be returned by project service. indexId field is enough for filtering
const { isAppStreamConfigured } = await projectService.mustFind(requestContext, {
Expand Down Expand Up @@ -505,7 +501,7 @@ class EnvironmentScService extends Service {
);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await this.verifyAppStreamConfig(requestContext, existingEnvironment.id);
await this.verifyAppStreamConfig(requestContext, existingEnvironment.projectId);

const by = _.get(requestContext, 'principalIdentifier.uid');
const { id, rev } = environment;
Expand Down Expand Up @@ -659,9 +655,6 @@ class EnvironmentScService extends Service {
existingEnvironment,
);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await this.verifyAppStreamConfig(requestContext, existingEnvironment.id);

const { status, outputs, projectId } = existingEnvironment;

// expected environment run state based on operation
Expand Down Expand Up @@ -900,9 +893,6 @@ class EnvironmentScService extends Service {
existingEnvironment,
);

// Verify environment is linked to an AppStream project when application has AppStream enabled
await this.verifyAppStreamConfig(requestContext, existingEnvironment.id);

await this.update(requestContext, {
id,
rev: existingEnvironment.rev,
Expand Down