-
Notifications
You must be signed in to change notification settings - Fork 4
fix: Resolve the cloud provider when a resource search omits it #68
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
Changes from all commits
1dda008
ff31aaa
18a6e8b
2fe79f9
d0cbe22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { describe, it } from 'node:test'; | ||
| import type { AxiosRequestConfig } from 'axios'; | ||
| import type Env0Client from './env0-client'; | ||
| import type { CloudConfiguration } from './models/cloud-configuration'; | ||
| import { Env0Service } from './env0-service'; | ||
|
|
||
| const config = { | ||
| organizationId: 'org-1', | ||
| apiUrl: 'https://api.env0.com', | ||
| apiKeyId: 'id', | ||
| apiKeySecret: 'secret' | ||
| }; | ||
|
|
||
| const buildService = ( | ||
| providers: CloudConfiguration['provider'][] | ||
| ): { service: Env0Service; requests: AxiosRequestConfig[] } => { | ||
| const requests: AxiosRequestConfig[] = []; | ||
| const client = { | ||
| request: async (request: AxiosRequestConfig) => { | ||
| requests.push(request); | ||
| return request.url === '/mcp/cloud/configurations' | ||
| ? providers.map(provider => ({ provider })) | ||
| : { resources: [], total: 0 }; | ||
| } | ||
| } as unknown as Env0Client; | ||
|
|
||
| return { service: new Env0Service(config, client), requests }; | ||
| }; | ||
|
|
||
| describe('getCloudResources', () => { | ||
| it('fills in the organization cloud provider when the search has no provider and no configuration', async () => { | ||
| const { service, requests } = buildService(['GCP']); | ||
|
|
||
| await service.getCloudResources({ filters: {} }); | ||
|
|
||
| assert.deepEqual(requests.at(-1)?.data.filters, { cloudProvider: { eq: 'GCP' } }); | ||
| }); | ||
|
|
||
| it('asks for a provider when the organization has more than one', async () => { | ||
| const { service } = buildService(['AWS', 'GCP']); | ||
|
|
||
| await assert.rejects(service.getCloudResources({ filters: {} }), /AWS, GCP/); | ||
| }); | ||
|
|
||
| it('says so when the organization has no cloud configurations', async () => { | ||
| const { service } = buildService([]); | ||
|
|
||
| await assert.rejects( | ||
| service.getCloudResources({ filters: {} }), | ||
| /No cloud configurations found/ | ||
| ); | ||
| }); | ||
|
|
||
| it('keeps the search as is when it already has a configuration ID', async () => { | ||
| const { service, requests } = buildService(['AWS']); | ||
| const filters = { cloudConfigurationId: { eq: 'config-1' } }; | ||
|
|
||
| await service.getCloudResources({ filters }); | ||
|
|
||
| assert.deepEqual(requests.at(-1)?.data.filters, filters); | ||
| assert.equal(requests.length, 1); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import _ from 'lodash'; | ||
| import type { AbortEnvironmentParams } from '../mcp/schemas/abort-environment-schema'; | ||
| import type { ApproveEnvironmentParams } from '../mcp/schemas/approve-environment-schema'; | ||
| import type { CancelEnvironmentParams } from '../mcp/schemas/cancel-environment-schema'; | ||
|
|
@@ -50,16 +51,41 @@ export class Env0Service { | |
| } | ||
|
|
||
| async getCloudResources(params: GetCloudResourcesParams): Promise<CloudResourcesResponse> { | ||
| const filters = await this.withCloudProvider(params.filters); | ||
|
|
||
| return this.env0Client.request<CloudResourcesResponse>({ | ||
| url: '/mcp/cloud/resources', | ||
| method: 'POST', | ||
| data: { | ||
| organizationId: this.config.organizationId || undefined, | ||
| ...params | ||
| ...params, | ||
| filters | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // The API rejects a search that has neither cloudConfigurationId nor cloudProvider, and callers often send neither. | ||
| private async withCloudProvider( | ||
| filters: GetCloudResourcesParams['filters'] | ||
| ): Promise<GetCloudResourcesParams['filters']> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard checks Either check
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 18a6e8b by dropping |
||
| if (filters.cloudConfigurationId?.eq || filters.cloudProvider?.eq) return filters; | ||
|
|
||
| const providers = _.uniq((await this.getCloudConfigurations()).map(({ provider }) => provider)); | ||
| const [provider] = providers; | ||
|
|
||
| if (!provider) { | ||
| throw new Error('No cloud configurations found for this organization.'); | ||
| } | ||
|
|
||
| if (providers.length > 1) { | ||
| throw new Error( | ||
| `Set filters.cloudProvider.eq to one of: ${providers.join(', ')}, or set filters.cloudConfigurationId.eq.` | ||
| ); | ||
| } | ||
|
|
||
| return { ...filters, cloudProvider: { eq: provider } }; | ||
| } | ||
|
|
||
| async getProjects(): Promise<object[]> { | ||
| return this.env0Client.request({ | ||
| url: '/mcp/projects', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: no case for the zero-configurations path ("No cloud configurations found"). One more
buildService([])+assert.rejectswould cover it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 2fe79f9.