diff --git a/.changeset/3030.md b/.changeset/3030.md new file mode 100644 index 0000000000..89730dc14c --- /dev/null +++ b/.changeset/3030.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': minor +--- + +Add the ability to import components from Bitbucket Server to the service catalog diff --git a/.changeset/3066.md b/.changeset/3066.md new file mode 100644 index 0000000000..7bb67e70d1 --- /dev/null +++ b/.changeset/3066.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +Filters passed to the `/entities` endpoint of the catalog has changed format. + +The old way was to pass things on the form `?a=b&c=d`; the new way is to pass +things on the form `?filter=a=b,c=d`. See discussion in +[#2910](https://github.com/spotify/backstage/issues/2910) for details. + +The comma separated items within a single filter have an AND between them. If +multiple such filters are passed, they have an OR between those item groups. diff --git a/.changeset/clean-phones-vanish.md b/.changeset/clean-phones-vanish.md new file mode 100644 index 0000000000..4c80b82f24 --- /dev/null +++ b/.changeset/clean-phones-vanish.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-cost-insights': patch +--- + +export test utilities for mocking context diff --git a/contrib/chart/backstage/templates/backend-secret.yaml b/contrib/chart/backstage/templates/backend-secret.yaml index dc65f5777a..b340f39d7c 100644 --- a/contrib/chart/backstage/templates/backend-secret.yaml +++ b/contrib/chart/backstage/templates/backend-secret.yaml @@ -19,4 +19,5 @@ stringData: GITLAB_TOKEN: {{ .Values.auth.gitlabToken }} AZURE_TOKEN: {{ .Values.auth.azure.api.token }} NEW_RELIC_REST_API_KEY: {{ .Values.auth.newRelicRestApiKey }} + TRAVISCI_AUTH_TOKEN: {{ .Values.auth.travisciAuthToken }} {{- end }} diff --git a/contrib/chart/backstage/values.yaml b/contrib/chart/backstage/values.yaml index f92e8aad47..eb06422d34 100644 --- a/contrib/chart/backstage/values.yaml +++ b/contrib/chart/backstage/values.yaml @@ -249,3 +249,4 @@ auth: githubToken: g gitlabToken: g newRelicRestApiKey: r + travisciAuthToken: fake-travis-ci-auth-token diff --git a/docs/features/software-catalog/descriptor-format.md b/docs/features/software-catalog/descriptor-format.md index 3a1db235a0..62ac9da8d8 100644 --- a/docs/features/software-catalog/descriptor-format.md +++ b/docs/features/software-catalog/descriptor-format.md @@ -12,7 +12,7 @@ This both applies to objects given to and returned from the software catalog API, as well as to the descriptor files that the software catalog can ingest natively. In the API request/response cycle, a JSON representation is used, while the descriptor files are on YAML format to be more easily maintainable by -humans. However, the structure and semantics is the same in both cases. +humans. However, the structure and semantics are the same in both cases. Although it's possible to name catalog entity descriptor files however you wish, we recommend that you name them `catalog-info.yaml`. @@ -113,7 +113,7 @@ data. Backstage specific entities have an `apiVersion` that is prefixed with `backstage.io/`, to distinguish them from other types of object that share the same type of structure. This may be relevant when co-hosting these -specifications with e.g. kubernetes object manifests, or when an organization +specifications with e.g. Kubernetes object manifests, or when an organization adds their own specific kinds of entity to the catalog. Early versions of the catalog will be using alpha/beta versions, e.g. diff --git a/docs/features/techdocs/creating-and-publishing.md b/docs/features/techdocs/creating-and-publishing.md index 947e6de371..38f452dcf3 100644 --- a/docs/features/techdocs/creating-and-publishing.md +++ b/docs/features/techdocs/creating-and-publishing.md @@ -30,7 +30,9 @@ scratch. Your working Backstage instance should by default have a documentation template added. If not, follow these [instructions](../software-templates/installation.md#adding-templates) to add -the documentation template. +the documentation template. The template creates a component with only TechDocs +configuration and default markdown files as below mentioned in manual +documentation setup, and is otherwise empty. ![Documentation Template](../../assets/techdocs/documentation-template.png) diff --git a/docs/getting-started/create-an-app.md b/docs/getting-started/create-an-app.md index 670c7fbf98..c35eded6a5 100644 --- a/docs/getting-started/create-an-app.md +++ b/docs/getting-started/create-an-app.md @@ -13,7 +13,7 @@ need to run Backstage in your own environment. ## Create an app To create a Backstage app, you will need to have -[NodeJS](https://nodejs.org/en/download/) Active LTS Release installed +[Node.js](https://nodejs.org/en/download/) Active LTS Release installed (currently v12). Backstage provides a utility for creating new apps. It guides you through the diff --git a/docs/getting-started/deployment-helm.md b/docs/getting-started/deployment-helm.md index e674d28742..c062ed3951 100644 --- a/docs/getting-started/deployment-helm.md +++ b/docs/getting-started/deployment-helm.md @@ -8,10 +8,10 @@ sidebar_label: Kubernetes and Helm # Helm charts An example Backstage app can be deployed in Kubernetes using the -[Backstage Helm charts](https://github.com/spotify/backstage/tree/master/contrib/chart/backstage) +[Backstage Helm charts](https://github.com/spotify/backstage/tree/master/contrib/chart/backstage). -First, choose a DNS name where backstage will be hosted create a yaml file for -your custom configuration. +First, choose a DNS name where Backstage will be hosted, and create a YAML file +for your custom configuration. ```yaml appConfig: @@ -31,7 +31,7 @@ appConfig: Then use it to run: -``` +```bash git clone https://github.com/spotify/backstage.git cd contrib/chart/backstage helm dependency update diff --git a/docs/getting-started/deployment-k8s.md b/docs/getting-started/deployment-k8s.md index 69aa59a600..f123dd2548 100644 --- a/docs/getting-started/deployment-k8s.md +++ b/docs/getting-started/deployment-k8s.md @@ -4,7 +4,7 @@ title: Kubernetes description: Documentation on Kubernetes and K8s Deployment --- -Backstage itself provides tooling up to the point of building docker images. +Backstage itself provides tooling up to the point of building Docker images. Beyond that point we do not have an opinionated way to deploy Backstage within Kubernetes, as each cluster has its own unique set of tooling and patterns. diff --git a/docs/getting-started/deployment-other.md b/docs/getting-started/deployment-other.md index 3e06e17c77..cfd2e903d9 100644 --- a/docs/getting-started/deployment-other.md +++ b/docs/getting-started/deployment-other.md @@ -23,7 +23,7 @@ Then open http://localhost/ on your browser. Deploying to Heroku is relatively easy following these steps. First, make sure you have the -[heroku CLI installed](https://devcenter.heroku.com/articles/heroku-cli) and log +[Heroku CLI installed](https://devcenter.heroku.com/articles/heroku-cli) and log into it as well as login into Heroku's [container registry](https://devcenter.heroku.com/articles/container-registry-and-runtime). @@ -32,7 +32,7 @@ $ heroku login $ heroku container:login ``` -You _might_ also need to set your Heroku app's stack to `container` +You _might_ also need to set your Heroku app's stack to `container`. ```bash $ heroku stack:set container -a diff --git a/docs/overview/architecture-overview.md b/docs/overview/architecture-overview.md index 504d6ea2c9..ee2d3c6097 100644 --- a/docs/overview/architecture-overview.md +++ b/docs/overview/architecture-overview.md @@ -24,7 +24,7 @@ different ways. ## Overview The following diagram shows how Backstage might look when deployed inside a -company which uses the Tech Radar plugin, the Lighthouse plugin, the Circle CI +company which uses the Tech Radar plugin, the Lighthouse plugin, the CircleCI plugin and the service catalog. There are 3 main components in this architecture: @@ -53,9 +53,9 @@ example, the Lighthouse plugin is registered with the UI on `/lighthouse`. ![The lighthouse plugin UI](../assets/architecture-overview/lighthouse-plugin.png) -The Circle CI plugin is available on `/circleci`. +The CircleCI plugin is available on `/circleci`. -![Circle CI Plugin UI](../assets/architecture-overview/circle-ci.png) +![CircleCI Plugin UI](../assets/architecture-overview/circle-ci.png) ## Plugins and plugin backends @@ -154,29 +154,28 @@ Cross Origin Resource Sharing policies which prevent a browser page served at [https://example.com](https://example.com) from serving resources hosted at https://circleci.com. -![CircleCi plugin talking to proxy talking to SaaS Circle CI](../assets/architecture-overview/circle-ci-plugin-architecture.png) +![CircleCI plugin talking to proxy talking to SaaS Circle CI](../assets/architecture-overview/circle-ci-plugin-architecture.png) ## Databases -As we have seen, both the lighthouse-audit-service and catalog-backend require a -database to work with. +As we have seen, both the `lighthouse-audit-service` and `catalog-backend` +require a database to work with. -At the time of writing, the lighthouse-audit-service requires PostgreSQL to work -with. The service catalog backend uses an in-memory Sqlite3 instance. This is a -development oriented setup and there are plans to support other databases in the -future. +At the time of writing, the `lighthouse-audit-service` requires PostgreSQL to +work with. The service catalog backend uses an in-memory Sqlite3 instance. This +is a development-oriented setup and there are plans to support other databases +in the future. -To learn more about the future of databases and Backstage, see the following two +To learn more about the future of databases and Backstage, see the following GitHub issues. -[Knex + Plugins (Multiple vs Single Database) · Issue #1598 · spotify/backstage](https://github.com/spotify/backstage/issues/1598) - -[Update migrations to support postgres by dariddler · Pull Request #1527 · spotify/backstage](https://github.com/spotify/backstage/pull/1527#discussion_r450374145) +- [Knex + Plugins (Multiple vs Single Database) · Issue #1598 · spotify/backstage](https://github.com/spotify/backstage/issues/1598) +- [Update migrations to support postgres by dariddler · Pull Request #1527 · spotify/backstage](https://github.com/spotify/backstage/pull/1527#discussion_r450374145) ## Containerization The example Backstage architecture shown above would Dockerize into three -separate docker images. +separate Docker images. 1. The frontend container 2. The backend container diff --git a/docs/plugins/call-existing-api.md b/docs/plugins/call-existing-api.md index 14ea4c1f5a..3fff90c3f6 100644 --- a/docs/plugins/call-existing-api.md +++ b/docs/plugins/call-existing-api.md @@ -88,7 +88,7 @@ The proxy is powered by the `http-proxy-middleware` package. See [Proxying](proxying.md) for a full description of its configuration options. Internally at Spotify, the proxy option has been the overwhelmingly most popular -choice for plugin makers. Since we have DNS based service discovery in place and +choice for plugin makers. Since we have DNS-based service discovery in place and a microservices framework that made it trivial to expose plain HTTP, it has been a matter of just adding a few lines of Backstage config to get the benefit of being easily and robustly reachable from users' web browsers as well. diff --git a/docs/plugins/testing.md b/docs/plugins/testing.md index c6e1599786..e60b5fa8b7 100644 --- a/docs/plugins/testing.md +++ b/docs/plugins/testing.md @@ -248,15 +248,15 @@ Testing an API involves verifying four things: ### Mocking API Calls -[Mocking in jest](https://facebook.github.io/jest/docs/en/mock-functions.html) +[Mocking in Jest](https://facebook.github.io/jest/docs/en/mock-functions.html) involves wrapping existing functions (like an API call function) with an alternative. For example: -**./Api.js** +**`./MyApi.js`** -``` +```js export { fetchSomethingFromServer: () => { // Live production call to a URI. Must be avoided during testing! @@ -265,9 +265,9 @@ export { }; ``` -**./\_\_mocks\_\_/Api.js** +**`./\_\_mocks\_\_/MyApi.js`** -``` +```js export { fetchSomethingFromServer: () => { // Simulate a production call, but avoid jest and just use a promise @@ -276,16 +276,16 @@ export { } ``` -**./Api.test.js** +**`./MyApi.test.js`** -``` +```js /* eslint-disable import/first */ jest.mock('./MyApi'); // Instruct Jest to swap all future imports of './MyApi.js' to './__mocks__/MyApi.js' import MyApi from './MyApi'; // Will actually return the contents of the file in the __mocks__ folder now -it ('loads data', (done) => { +it('loads data', done => { MyApi.fetchSomethingFromServer().then(result => { expect(result).toBe('some result object simulating server data here'); done(); diff --git a/docs/support/project-structure.md b/docs/support/project-structure.md index fd8e76d58a..9720c4db3e 100644 --- a/docs/support/project-structure.md +++ b/docs/support/project-structure.md @@ -41,10 +41,10 @@ the code. appreciate contributions in here and encourage them being kept up to date. - [`docs/`](https://github.com/spotify/backstage/tree/master/docs) - This is - where we keep all of our documentation markdown files. These ends up on - http://backstage.io/docs. Just keep in mind that changes to - [this](https://github.com/spotify/backstage/blob/master/microsite/sidebars.json) - file also needs to be updated. + where we keep all of our documentation Markdown files. These ends up on + http://backstage.io/docs. Just keep in mind that changes to the + [`sidebars.json`](https://github.com/spotify/backstage/blob/master/microsite/sidebars.json) + file may be needed as sections are added/removed. - [`.editorconfig`](https://github.com/spotify/backstage/tree/master/.editorconfig) - A configuration file used by most common code editors. @@ -111,13 +111,13 @@ are separated out into their own folder, see further down. We also want as few dependencies as possible to reduce download time when running the cli which is another reason this is a separate package. -* [`config/`](https://github.com/spotify/backstage/tree/master/packages/config) - +- [`config/`](https://github.com/spotify/backstage/tree/master/packages/config) - The way we read configuration data. This package can take a bunch of config objects and merge them together. [app-config.yaml](https://github.com/spotify/backstage/blob/master/app-config.yaml) is an example of an config object. -* [`config-loader/`](https://github.com/spotify/backstage/tree/master/packages/config-loader) - +- [`config-loader/`](https://github.com/spotify/backstage/tree/master/packages/config-loader) - This package is used to read config objects. It does not know how to merge, but only reads files and passes them on to the config. As this part is only used by the backend, we chose to separate `config` and `config-loader` into @@ -130,21 +130,21 @@ are separated out into their own folder, see further down. Apart from that it re-exports everything from [`core-api`] so that users only need to rely on one package. -* [`core-api/`](https://github.com/spotify/backstage/tree/master/packages/core-api) - +- [`core-api/`](https://github.com/spotify/backstage/tree/master/packages/core-api) - This package contains APIs and definitions of such. It is it's own package because we needed to split our `test-utils` package. It's an implementation detail that we try to hide from our users, and no one should have to depend on it directly. -* [`test-utils/`](https://github.com/spotify/backstage/tree/master/packages/test-utils) - +- [`test-utils/`](https://github.com/spotify/backstage/tree/master/packages/test-utils) - This package contains specific testing facilities used when testing `core-api`. -* [`test-utils-core/`](https://github.com/spotify/backstage/tree/master/packages/test-utils-core) - +- [`test-utils-core/`](https://github.com/spotify/backstage/tree/master/packages/test-utils-core) - This package contains more general purpose testing facilities for testing a Backstage App. -* [`create-app/`](https://github.com/spotify/backstage/tree/master/packages/create-app) - +- [`create-app/`](https://github.com/spotify/backstage/tree/master/packages/create-app) - An CLI to specifically scaffold a new Backstage App. It does so by using a [template](https://github.com/spotify/backstage/tree/master/packages/create-app/templates/default-app). @@ -152,32 +152,32 @@ are separated out into their own folder, see further down. Helps you setup a plugin for isolated development so that it can be served separately. -* [`docgen/`](https://github.com/spotify/backstage/tree/master/packages/docgen) - +- [`docgen/`](https://github.com/spotify/backstage/tree/master/packages/docgen) - Uses the [Typescript Compiler API](https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API) to read out definitions and generate documentation for it. -* [`e2e-test/`](https://github.com/spotify/backstage/tree/master/packages/e2e-test) - +- [`e2e-test/`](https://github.com/spotify/backstage/tree/master/packages/e2e-test) - Another CLI that can be run to try out what would happen if you built all the packages, publish them, created a new app, and the run it. CI uses this for e2e-tests. -* [`storybook/`](https://github.com/spotify/backstage/tree/master/packages/storybook) - +- [`storybook/`](https://github.com/spotify/backstage/tree/master/packages/storybook) - This folder contains only the storybook config. Stories are within the core package. The Backstage Storybook is found [here](https://backstage.io/storybook) -* [`techdocs-cli/`](https://github.com/spotify/backstage/tree/master/packages/techdocs-cli) - +- [`techdocs-cli/`](https://github.com/spotify/backstage/tree/master/packages/techdocs-cli) - Used for verifying TechDocs locally. -* [`techdocs-container/`](https://github.com/spotify/backstage/tree/master/packages/techdocs-container) - +- [`techdocs-container/`](https://github.com/spotify/backstage/tree/master/packages/techdocs-container) - Used by the `techdocs-cli` -* [`test-utils-core/`](https://github.com/spotify/backstage/tree/master/packages/test-utils-core) +- [`test-utils-core/`](https://github.com/spotify/backstage/tree/master/packages/test-utils-core) -* [`test-utils/`](https://github.com/spotify/backstage/tree/master/packages/test-utils) +- [`test-utils/`](https://github.com/spotify/backstage/tree/master/packages/test-utils) -* [`theme/`](https://github.com/spotify/backstage/tree/master/packages/theme) - +- [`theme/`](https://github.com/spotify/backstage/tree/master/packages/theme) - Holds the Backstage Theme. ### `plugins/` diff --git a/docs/tutorials/journey.md b/docs/tutorials/journey.md index 059782de33..7627920a75 100644 --- a/docs/tutorials/journey.md +++ b/docs/tutorials/journey.md @@ -239,12 +239,12 @@ hits merge. # 8. Attack of the Clones Sam just released v1.8.4 of the plugin, and at this point it's so popular that a -couple of other plugins has started depending on the `sam.wise/spotify-track-id` -annotation. One such plugin being the `spotify-album-art` plugin that can -display the album art of the theme tune as the background of the entity header. -Sam thinks it's all pretty cool, but doesn't like that the annotation that was -once an internal concern of the plugin is now becoming a standard in the -community. +couple of other plugins have started depending on the +`sam.wise/spotify-track-id` annotation. One such plugin being the +`spotify-album-art` plugin that can display the album art of the theme tune as +the background of the entity header. Sam thinks it's all pretty cool, but +doesn't like that the annotation that was once an internal concern of the plugin +is now becoming a standard in the community. In order to standardize the annotation in Backstage, Sam submits a Pull Request to the Backstage Core repo. The request suggests a new well-known metadata diff --git a/docs/tutorials/quickstart-app-auth.md b/docs/tutorials/quickstart-app-auth.md index 7fc7e739f2..1dcca9d7e0 100644 --- a/docs/tutorials/quickstart-app-auth.md +++ b/docs/tutorials/quickstart-app-auth.md @@ -7,7 +7,7 @@ title: Monorepo App Setup With Authentication
-> This document takes you through setting up a backstage app that runs in your +> This document takes you through setting up a Backstage app that runs in your > own environment. It starts with a skeleton install and verifying of the > monorepo's functionality. Next, GitHub authentication is added and tested. > diff --git a/microsite/siteConfig.js b/microsite/siteConfig.js index 370d491eb3..06ae8453a0 100644 --- a/microsite/siteConfig.js +++ b/microsite/siteConfig.js @@ -60,7 +60,7 @@ const siteConfig = { /* path to images for header/footer */ // headerIcon: "img/android-chrome-192x192.png", footerIcon: 'img/android-chrome-192x192.png', - favicon: 'img/favicon.svg', + favicon: 'img/favicon.ico', /* Colors for website */ colors: { diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 5a2357f74e..875163dd96 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -41,7 +41,7 @@ "express": "^4.17.1", "express-prom-bundle": "^6.1.0", "express-promise-router": "^3.0.3", - "git-url-parse": "^11.3.0", + "git-url-parse": "^11.4.0", "helmet": "^4.0.0", "knex": "^0.21.1", "lodash": "^4.17.15", diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index bd7c43d887..210ee16873 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -14,139 +14,167 @@ * limitations under the License. */ -import { rest } from 'msw'; -import { setupServer } from 'msw/node'; import { ConfigReader } from '@backstage/config'; -import { getVoidLogger } from '../logging'; -import { BitbucketUrlReader } from './BitbucketUrlReader'; -import { msw } from '@backstage/test-utils'; - -const logger = getVoidLogger(); +import { + BitbucketUrlReader, + getApiRequestOptions, + getApiUrl, + ProviderConfig, + readConfig, +} from './BitbucketUrlReader'; describe('BitbucketUrlReader', () => { - const worker = setupServer(); + describe('getApiRequestOptions', () => { + it('inserts a token when needed', () => { + const withToken: ProviderConfig = { + host: '', + apiBaseUrl: '', + token: 'A', + }; + const withoutToken: ProviderConfig = { + host: '', + apiBaseUrl: '', + }; + expect( + (getApiRequestOptions(withToken).headers as any).Authorization, + ).toEqual('Bearer A'); + expect( + (getApiRequestOptions(withoutToken).headers as any).Authorization, + ).toBeUndefined(); + }); - msw.setupDefaultHandlers(worker); + it('insert basic auth when needed', () => { + const withUsernameAndPassword: ProviderConfig = { + host: '', + apiBaseUrl: '', + username: 'some-user', + appPassword: 'my-secret', + }; + const withoutUsernameAndPassword: ProviderConfig = { + host: '', + apiBaseUrl: '', + }; + expect( + (getApiRequestOptions(withUsernameAndPassword).headers as any) + .Authorization, + ).toEqual('Basic c29tZS11c2VyOm15LXNlY3JldA=='); + expect( + (getApiRequestOptions(withoutUsernameAndPassword).headers as any) + .Authorization, + ).toBeUndefined(); + }); + }); - beforeEach(() => { - worker.use( - rest.get('*', (req, res, ctx) => - res( - ctx.status(200), - ctx.json({ - url: req.url.toString(), - headers: req.headers.getAllHeaders(), - }), + describe('getApiUrl', () => { + it('rejects targets that do not look like URLs', () => { + const config: ProviderConfig = { host: '', apiBaseUrl: '' }; + expect(() => getApiUrl('a/b', config)).toThrow(/Incorrect URL: a\/b/); + }); + it('happy path for Bitbucket Cloud', () => { + const config: ProviderConfig = { + host: 'bitbucket.org', + apiBaseUrl: 'https://api.bitbucket.org/2.0', + }; + expect( + getApiUrl( + 'https://bitbucket.org/org-name/repo-name/src/master/templates/my-template.yaml', + config, ), - ), - ); - }); - - const createConfig = (username?: string, appPassword?: string) => - new ConfigReader( - { - integrations: { - bitbucket: [ - { - host: 'bitbucket.org', - username: username, - appPassword: appPassword, - }, - ], - }, - }, - 'test-config', - ); - - it.each([ - { - url: - 'https://bitbucket.org/org-name/repo-name/src/master/templates/my-template.yaml', - config: createConfig(), - response: expect.objectContaining({ - url: + ).toEqual( + new URL( 'https://api.bitbucket.org/2.0/repositories/org-name/repo-name/src/master/templates/my-template.yaml', - }), - }, - { - url: - 'https://bitbucket.org/org-name/repo-name/src/master/templates/my-template.yaml', - config: createConfig('some-user', 'my-secret'), - response: expect.objectContaining({ - headers: expect.objectContaining({ - authorization: 'Basic c29tZS11c2VyOm15LXNlY3JldA==', - }), - }), - }, - { - url: - 'https://bitbucket.org/org-name/repo-name/src/master/templates/my-template.yaml', - config: createConfig(), - response: expect.objectContaining({ - headers: expect.not.objectContaining({ - authorization: expect.anything(), - }), - }), - }, - { - url: - 'https://bitbucket.org/org-name/repo-name/src/master/templates/my-template.yaml', - config: createConfig(undefined, 'only-password-provided'), - response: expect.objectContaining({ - headers: expect.not.objectContaining({ - authorization: expect.anything(), - }), - }), - }, - ])('should handle happy path %#', async ({ url, config, response }) => { - const [{ reader }] = BitbucketUrlReader.factory({ config, logger }); - - const data = await reader.read(url); - const res = await JSON.parse(data.toString('utf-8')); - expect(res).toEqual(response); + ), + ); + }); + it('happy path for Bitbucket Server', () => { + const config: ProviderConfig = { + host: 'bitbucket.mycompany.net', + apiBaseUrl: 'https://bitbucket.mycompany.net/rest/api/1.0', + }; + expect( + getApiUrl( + 'https://bitbucket.mycompany.net/projects/a/repos/b/browse/path/to/c.yaml', + config, + ), + ).toEqual( + new URL( + 'https://bitbucket.mycompany.net/rest/api/1.0/projects/a/repos/b/raw/path/to/c.yaml', + ), + ); + }); }); - it.each([ - { - url: 'https://api.com/a/b/blob/master/path/to/c.yaml', - config: createConfig(), - error: - 'Incorrect url: https://api.com/a/b/blob/master/path/to/c.yaml, Error: Wrong Bitbucket URL or Invalid file path', - }, - { - url: 'com/a/b/blob/master/path/to/c.yaml', - config: createConfig(), - error: - 'Incorrect url: com/a/b/blob/master/path/to/c.yaml, TypeError: Invalid URL: com/a/b/blob/master/path/to/c.yaml', - }, - { - url: '', - config: createConfig('', ''), - error: - "Invalid type in config for key 'integrations.bitbucket[0].username' in 'test-config', got empty-string, wanted string", - }, - { - url: '', - config: createConfig('only-user-provided', ''), - error: - "Invalid type in config for key 'integrations.bitbucket[0].appPassword' in 'test-config', got empty-string, wanted string", - }, - { - url: '', - config: createConfig('', 'only-password-provided'), - error: - "Invalid type in config for key 'integrations.bitbucket[0].username' in 'test-config', got empty-string, wanted string", - }, - { - url: '', - config: createConfig('only-user-provided', undefined), - error: - "Missing required config value at 'integrations.bitbucket[0].appPassword'", - }, - ])('should handle error path %#', async ({ url, config, error }) => { - await expect(async () => { - const [{ reader }] = BitbucketUrlReader.factory({ config, logger }); - await reader.read(url); - }).rejects.toThrow(error); + describe('readConfig', () => { + function config( + providers: { + host: string; + apiBaseUrl?: string; + token?: string; + username?: string; + password?: string; + }[], + ) { + return ConfigReader.fromConfigs([ + { + context: '', + data: { + integrations: { bitbucket: providers }, + }, + }, + ]); + } + + it('adds a default Bitbucket Cloud entry when missing', () => { + const output = readConfig(config([])); + expect(output).toEqual([ + { + host: 'bitbucket.org', + apiBaseUrl: 'https://api.bitbucket.org/2.0', + }, + ]); + }); + + it('injects the correct Bitbucket Cloud API base URL when missing', () => { + const output = readConfig(config([{ host: 'bitbucket.org' }])); + expect(output).toEqual([ + { + host: 'bitbucket.org', + apiBaseUrl: 'https://api.bitbucket.org/2.0', + }, + ]); + }); + + it('rejects custom targets with no base URLs', () => { + expect(() => + readConfig(config([{ host: 'bitbucket.mycompany.net' }])), + ).toThrow( + "Bitbucket integration for 'bitbucket.mycompany.net' must configure an explicit apiBaseUrl", + ); + }); + + it('rejects funky configs', () => { + expect(() => readConfig(config([{ host: 7 } as any]))).toThrow(/host/); + expect(() => readConfig(config([{ token: 7 } as any]))).toThrow(/token/); + expect(() => + readConfig(config([{ host: 'bitbucket.org', apiBaseUrl: 7 } as any])), + ).toThrow(/apiBaseUrl/); + expect(() => + readConfig(config([{ host: 'bitbucket.org', token: 7 } as any])), + ).toThrow(/token/); + }); + }); + + describe('implementation', () => { + it('rejects unknown targets', async () => { + const processor = new BitbucketUrlReader({ + host: 'bitbucket.org', + apiBaseUrl: 'https://api.bitbucket.org/2.0', + }); + await expect( + processor.read('https://not.bitbucket.com/apa'), + ).rejects.toThrow( + 'Incorrect URL: https://not.bitbucket.com/apa, Error: Invalid Bitbucket URL or file path', + ); + }); }); }); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index bf07dc18a6..348426fd34 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -14,71 +14,186 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; import { Config } from '@backstage/config'; -import { ReaderFactory, UrlReader } from './types'; +import parseGitUri from 'git-url-parse'; +import fetch from 'cross-fetch'; import { NotFoundError } from '../errors'; +import { ReaderFactory, UrlReader } from './types'; -type Options = { - // TODO: added here for future support, but we only allow bitbucket.org for now +const DEFAULT_BASE_URL = 'https://api.bitbucket.org/2.0'; + +/** + * The configuration parameters for a single Bitbucket API provider. + */ +export type ProviderConfig = { + /** + * The host of the target that this matches on, e.g. "bitbucket.com" + */ host: string; - auth?: { - username: string; - appPassword: string; - }; + + /** + * The base URL of the API of this provider, e.g. "https://api.bitbucket.org/2.0", + * with no trailing slash. + * + * May be omitted specifically for Bitbucket Cloud; then it will be deduced. + * + * The API will always be preferred if both its base URL and a token are + * present. + */ + apiBaseUrl?: string; + + /** + * The authorization token to use for requests to a Bitbucket Server provider. + * + * See https://confluence.atlassian.com/bitbucketserver/personal-access-tokens-939515499.html + * + * If no token is specified, anonymous access is used. + */ + token?: string; + + /** + * The username to use for requests to Bitbucket Cloud (bitbucket.org). + */ + username?: string; + + /** + * Authentication with Bitbucket Cloud (bitbucket.org) is done using app passwords. + * + * See https://support.atlassian.com/bitbucket-cloud/docs/app-passwords/ + */ + appPassword?: string; }; -function readConfig(config: Config): Options[] { - const optionsArr = Array(); +export function getApiRequestOptions(provider: ProviderConfig): RequestInit { + const headers: HeadersInit = {}; + + if (provider.token) { + headers.Authorization = `Bearer ${provider.token}`; + } else if (provider.username && provider.appPassword) { + headers.Authorization = `Basic ${Buffer.from( + `${provider.username}:${provider.appPassword}`, + 'utf8', + ).toString('base64')}`; + } + + return { + headers, + }; +} + +// Converts for example +// from: https://bitbucket.org/orgname/reponame/src/master/file.yaml +// to: https://api.bitbucket.org/2.0/repositories/orgname/reponame/src/master/file.yaml +export function getApiUrl(target: string, provider: ProviderConfig): URL { + try { + const { owner, name, ref, filepathtype, filepath } = parseGitUri(target); + if ( + !owner || + !name || + (filepathtype !== 'browse' && + filepathtype !== 'raw' && + filepathtype !== 'src') + ) { + throw new Error('Invalid Bitbucket URL or file path'); + } + + const pathWithoutSlash = filepath.replace(/^\//, ''); + + if (provider.host === 'bitbucket.org') { + if (!ref) { + throw new Error('Invalid Bitbucket URL or file path'); + } + return new URL( + `${provider.apiBaseUrl}/repositories/${owner}/${name}/src/${ref}/${pathWithoutSlash}`, + ); + } + return new URL( + `${provider.apiBaseUrl}/projects/${owner}/repos/${name}/raw/${pathWithoutSlash}?at=${ref}`, + ); + } catch (e) { + throw new Error(`Incorrect URL: ${target}, ${e}`); + } +} + +export function readConfig(config: Config): ProviderConfig[] { + const providers: ProviderConfig[] = []; const providerConfigs = config.getOptionalConfigArray('integrations.bitbucket') ?? []; + // First read all the explicit providers for (const providerConfig of providerConfigs) { const host = providerConfig.getOptionalString('host') ?? 'bitbucket.org'; + let apiBaseUrl = providerConfig.getOptionalString('apiBaseUrl'); + const token = providerConfig.getOptionalString('token'); + const username = providerConfig.getOptionalString('username'); + const appPassword = providerConfig.getOptionalString('appPassword'); - let auth; - if (providerConfig.has('username')) { - const username = providerConfig.getString('username'); - const appPassword = providerConfig.getString('appPassword'); - auth = { username, appPassword }; + if (apiBaseUrl) { + apiBaseUrl = apiBaseUrl.replace(/\/+$/, ''); + } else if (host === 'bitbucket.org') { + apiBaseUrl = DEFAULT_BASE_URL; } - optionsArr.push({ host, auth }); + if (!apiBaseUrl) { + throw new Error( + `Bitbucket integration for '${host}' must configure an explicit apiBaseUrl`, + ); + } + if (!token && username && !appPassword) { + throw new Error( + `Bitbucket integration for '${host}' has configured a username but is missing a required appPassword.`, + ); + } + + providers.push({ + host, + apiBaseUrl, + token, + username, + appPassword, + }); } - // As a convenience we always make sure there's at least an unauthenticated - // reader for public bitbucket repos. - if (!optionsArr.some(p => p.host === 'bitbucket.org')) { - optionsArr.push({ host: 'bitbucket.org' }); + // If no explicit bitbucket.org provider was added, put one in the list as + // a convenience + if (!providers.some(p => p.host === 'bitbucket.org')) { + providers.push({ + host: 'bitbucket.org', + apiBaseUrl: DEFAULT_BASE_URL, + }); } - return optionsArr; + return providers; } +/** + * A processor that adds the ability to read files from Bitbucket v1 and v2 APIs, such as + * the one exposed by Bitbucket Cloud itself. + */ export class BitbucketUrlReader implements UrlReader { + private config: ProviderConfig; + static factory: ReaderFactory = ({ config }) => { - return readConfig(config).map(options => { - const reader = new BitbucketUrlReader(options); - const predicate = (url: URL) => url.host === options.host; + return readConfig(config).map(provider => { + const reader = new BitbucketUrlReader(provider); + const predicate = (url: URL) => url.host === provider.host; return { reader, predicate }; }); }; - constructor(private readonly options: Options) { - if (options.host !== 'bitbucket.org') { - throw Error( - `Bitbucket integration currently only supports 'bitbucket.org', tried to use host '${options.host}'`, - ); - } + constructor(config: ProviderConfig) { + this.config = config; } async read(url: string): Promise { - const builtUrl = this.buildRawUrl(url); + const bitbucketUrl = getApiUrl(url, this.config); + + const options = getApiRequestOptions(this.config); let response: Response; try { - response = await fetch(builtUrl.toString(), this.getRequestOptions()); + response = await fetch(bitbucketUrl.toString(), options); } catch (e) { throw new Error(`Unable to read ${url}, ${e}`); } @@ -87,76 +202,19 @@ export class BitbucketUrlReader implements UrlReader { return Buffer.from(await response.text()); } - const message = `${url} could not be read as ${builtUrl}, ${response.status} ${response.statusText}`; + const message = `${url} could not be read as ${bitbucketUrl}, ${response.status} ${response.statusText}`; if (response.status === 404) { throw new NotFoundError(message); } throw new Error(message); } - // Converts - // from: https://bitbucket.org/orgname/reponame/src/master/file.yaml - // to: https://api.bitbucket.org/2.0/repositories/orgname/reponame/src/master/file.yaml - private buildRawUrl(target: string): URL { - try { - const url = new URL(target); - - const [ - empty, - userOrOrg, - repoName, - srcKeyword, - ref, - ...restOfPath - ] = url.pathname.split('/'); - - if ( - url.hostname !== 'bitbucket.org' || - empty !== '' || - userOrOrg === '' || - repoName === '' || - srcKeyword !== 'src' - ) { - throw new Error('Wrong Bitbucket URL or Invalid file path'); - } - - // transform to api - url.pathname = [ - empty, - '2.0', - 'repositories', - userOrOrg, - repoName, - 'src', - ref, - ...restOfPath, - ].join('/'); - url.hostname = 'api.bitbucket.org'; - url.protocol = 'https'; - - return url; - } catch (e) { - throw new Error(`Incorrect url: ${target}, ${e}`); - } - } - - private getRequestOptions(): RequestInit { - const headers: HeadersInit = {}; - - if (this.options.auth) { - headers.Authorization = `Basic ${Buffer.from( - `${this.options.auth.username}:${this.options.auth.appPassword}`, - 'utf8', - ).toString('base64')}`; - } - - return { - headers, - }; - } - toString() { - const { host, auth } = this.options; - return `bitbucket{host=${host},authed=${Boolean(auth)}}`; + const { host, token, username, appPassword } = this.config; + let authed = Boolean(token); + if (!authed) { + authed = Boolean(username && appPassword); + } + return `bitbucket{host=${host},authed=${authed}}`; } } diff --git a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts index b6919a98c1..e56763f656 100644 --- a/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts +++ b/plugins/auth-backend/src/lib/catalog/CatalogIdentityClient.ts @@ -14,13 +14,13 @@ * limitations under the License. */ -import fetch from 'cross-fetch'; -import { UserEntity } from '@backstage/catalog-model'; import { ConflictError, NotFoundError, PluginEndpointDiscovery, } from '@backstage/backend-common'; +import { UserEntity } from '@backstage/catalog-model'; +import fetch from 'cross-fetch'; type UserQuery = { annotations: Record; @@ -42,15 +42,17 @@ export class CatalogIdentityClient { * Throws a NotFoundError or ConflictError if 0 or multiple users are found. */ async findUser(query: UserQuery): Promise { - const params = new URLSearchParams(); - params.append('kind', 'User'); - + const conditions = ['kind=user']; for (const [key, value] of Object.entries(query.annotations)) { - params.append(`metadata.annotations.${key}`, value); + const uk = encodeURIComponent(key); + const uv = encodeURIComponent(value); + conditions.push(`metadata.annotations.${uk}=${uv}`); } const baseUrl = await this.discovery.getBaseUrl('catalog'); - const response = await fetch(`${baseUrl}/entities?${params}`); + const response = await fetch( + `${baseUrl}/entities?filter=${conditions.join(',')}`, + ); if (!response.ok) { const text = await response.text(); diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index ecc154bc5a..7d41da2888 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -31,7 +31,7 @@ "express": "^4.17.1", "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", - "git-url-parse": "^11.3.0", + "git-url-parse": "^11.4.0", "knex": "^0.21.1", "ldapjs": "^2.2.0", "lodash": "^4.17.15", diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts index a801015915..6407000b6f 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.test.ts @@ -69,11 +69,13 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }); + expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': ['c'], + }, + ]); expect(db.setRelations).toHaveBeenCalledTimes(1); expect(db.setRelations).toHaveBeenCalledWith(expect.anything(), 'u', []); expect(db.addEntities).toHaveBeenCalledTimes(1); @@ -120,11 +122,13 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }); + expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': ['c'], + }, + ]); expect(db.entityByName).not.toHaveBeenCalled(); expect(db.entityByUid).toHaveBeenCalledTimes(1); expect(db.entityByUid).toHaveBeenCalledWith(expect.anything(), 'u'); @@ -194,11 +198,13 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }); + expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': ['c'], + }, + ]); expect(db.entityByName).toHaveBeenCalledTimes(1); expect(db.entityByName).toHaveBeenCalledWith(expect.anything(), { kind: 'b', @@ -254,11 +260,13 @@ describe('DatabaseEntitiesCatalog', () => { ]); expect(db.entities).toHaveBeenCalledTimes(1); - expect(db.entities).toHaveBeenCalledWith(expect.anything(), { - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': ['c'], - }); + expect(db.entities).toHaveBeenCalledWith(expect.anything(), [ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': ['c'], + }, + ]); expect(db.entityByName).not.toHaveBeenCalled(); expect(db.entityByUid).not.toHaveBeenCalled(); expect(db.updateEntity).not.toHaveBeenCalled(); diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 6c3065aeaa..947736dc06 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -60,7 +60,7 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { private readonly logger: Logger, ) {} - async entities(filters?: EntityFilters): Promise { + async entities(filters?: EntityFilters[]): Promise { const items = await this.database.transaction(tx => this.database.entities(tx, filters), ); @@ -109,9 +109,9 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const location = entityResponse.entity.metadata.annotations?.[LOCATION_ANNOTATION]; const colocatedEntities = location - ? await this.database.entities(tx, { - [`metadata.annotations.${LOCATION_ANNOTATION}`]: location, - }) + ? await this.database.entities(tx, [ + { [`metadata.annotations.${LOCATION_ANNOTATION}`]: location }, + ]) : [entityResponse]; for (const dbResponse of colocatedEntities) { await this.database.removeEntityByUid( @@ -234,11 +234,13 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { const markTimestamp = process.hrtime(); const names = requests.map(({ entity }) => entity.metadata.name); - const oldEntities = await this.entities({ - kind: kind, - 'metadata.namespace': namespace, - 'metadata.name': names, - }); + const oldEntities = await this.entities([ + { + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': names, + }, + ]); const oldEntitiesByName = new Map( oldEntities.map(e => [e.metadata.name, e]), diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 4d821c428a..539a0e531f 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity, Location, EntityRelationSpec } from '@backstage/catalog-model'; +import { Entity, EntityRelationSpec, Location } from '@backstage/catalog-model'; import type { EntityFilters } from '../database'; // @@ -31,7 +31,7 @@ export type EntityUpsertResponse = { }; export type EntitiesCatalog = { - entities(filters?: EntityFilters): Promise; + entities(filters?: EntityFilters[]): Promise; removeEntityByUid(uid: string): Promise; /** diff --git a/plugins/catalog-backend/src/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/database/CommonDatabase.test.ts index 0e59e0ff5a..ac11f1dd1a 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.test.ts @@ -347,7 +347,7 @@ describe('CommonDatabase', () => { await db.transaction(async tx => { await db.addEntities(tx, [{ entity: e1 }, { entity: e2 }]); }); - const result = await db.transaction(async tx => db.entities(tx, {})); + const result = await db.transaction(async tx => db.entities(tx, [])); expect(result.length).toEqual(2); expect(result).toEqual( expect.arrayContaining([ @@ -389,7 +389,7 @@ describe('CommonDatabase', () => { await expect( db.transaction(async tx => - db.entities(tx, { kind: 'k2', 'spec.c': 'some' }), + db.entities(tx, [{ kind: 'k2', 'spec.c': 'some' }]), ), ).resolves.toEqual([ { @@ -424,7 +424,7 @@ describe('CommonDatabase', () => { }); const rows = await db.transaction(async tx => - db.entities(tx, { apiVersion: 'a', 'spec.c': [null, 'some'] }), + db.entities(tx, [{ apiVersion: 'a', 'spec.c': [null, 'some'] }]), ); expect(rows.length).toEqual(3); @@ -471,7 +471,7 @@ describe('CommonDatabase', () => { }); const rows = await db.transaction(async tx => - db.entities(tx, { ApiVersioN: 'A', 'spEc.C': [null, 'some'] }), + db.entities(tx, [{ ApiVersioN: 'A', 'spEc.C': [null, 'some'] }]), ); expect(rows.length).toEqual(3); diff --git a/plugins/catalog-backend/src/database/CommonDatabase.ts b/plugins/catalog-backend/src/database/CommonDatabase.ts index e72926b13a..72e6bf0a5f 100644 --- a/plugins/catalog-backend/src/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/database/CommonDatabase.ts @@ -22,12 +22,12 @@ import { import { Entity, EntityName, + EntityRelationSpec, ENTITY_DEFAULT_NAMESPACE, ENTITY_META_GENERATED_FIELDS, generateEntityEtag, generateEntityUid, Location, - EntityRelationSpec, parseEntityName, } from '@backstage/catalog-model'; import Knex from 'knex'; @@ -218,66 +218,71 @@ export class CommonDatabase implements Database { async entities( txOpaque: unknown, - filters?: EntityFilters, + filters?: EntityFilters[], ): Promise { const tx = txOpaque as Knex.Transaction; let entitiesQuery = tx('entities'); - for (const [matchKey, matchVal] of Object.entries(filters ?? {})) { - const key = matchKey.toLowerCase().replace(/[*]/g, '%'); - const keyOp = key.includes('%') ? 'like' : '='; - const values = Array.isArray(matchVal) ? matchVal : [matchVal]; + for (const singleFilter of filters ?? []) { + entitiesQuery = entitiesQuery.orWhere(function singleFilterFn() { + for (const [matchKey, matchVal] of Object.entries(singleFilter)) { + const key = matchKey.toLowerCase().replace(/[*]/g, '%'); + const keyOp = key.includes('%') ? 'like' : '='; + const values = Array.isArray(matchVal) ? matchVal : [matchVal]; - let matchNulls = false; - const matchIn: string[] = []; - const matchLike: string[] = []; + let matchNulls = false; + const matchIn: string[] = []; + const matchLike: string[] = []; - for (const value of values) { - if (!value) { - matchNulls = true; - } else if (value.includes('*')) { - matchLike.push(value.toLowerCase().replace(/[*]/g, '%')); - } else { - matchIn.push(value.toLowerCase()); - } - } - - // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to - // make a lot of sense. However, it had abysmal performance on sqlite - // when datasets grew large, so we're using IN instead. - const matchQuery = tx('entities_search') - .select('entity_id') - .where(function keyFilter() { - this.andWhere('key', keyOp, key); - this.andWhere(function valueFilter() { - if (matchIn.length === 1) { - this.orWhere({ value: matchIn[0] }); - } else if (matchIn.length > 1) { - this.orWhereIn('value', matchIn); - } - if (matchLike.length) { - for (const x of matchLike) { - this.orWhere('value', 'like', tx.raw('?', [x])); - } + for (const value of values) { + if (!value) { + matchNulls = true; + } else if (value.includes('*')) { + matchLike.push(value.toLowerCase().replace(/[*]/g, '%')); + } else { + matchIn.push(value.toLowerCase()); } + } + + // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to + // make a lot of sense. However, it had abysmal performance on sqlite + // when datasets grew large, so we're using IN instead. + const matchQuery = tx('entities_search') + .select('entity_id') + .where(function keyFilter() { + this.andWhere('key', keyOp, key); + this.andWhere(function valueFilter() { + if (matchIn.length === 1) { + this.orWhere({ value: matchIn[0] }); + } else if (matchIn.length > 1) { + this.orWhereIn('value', matchIn); + } + if (matchLike.length) { + for (const x of matchLike) { + this.orWhere('value', 'like', tx.raw('?', [x])); + } + } + if (matchNulls) { + // Match explicit nulls, and then handle absence separately + // below + this.orWhereNull('value'); + } + }); + }); + + // Handle absence as nulls as well + this.andWhere(function match() { + this.whereIn('id', matchQuery); if (matchNulls) { - // Match explicit nulls, and then handle absence separately below - this.orWhereNull('value'); + this.orWhereNotIn( + 'id', + tx('entities_search') + .select('entity_id') + .where('key', keyOp, key), + ); } }); - }); - - // Handle absence as nulls as well - entitiesQuery = entitiesQuery.andWhere(function match() { - this.whereIn('id', matchQuery); - if (matchNulls) { - this.orWhereNotIn( - 'id', - tx('entities_search') - .select('entity_id') - .where('key', keyOp, key), - ); } }); } diff --git a/plugins/catalog-backend/src/database/types.ts b/plugins/catalog-backend/src/database/types.ts index fa038c0e1d..b8a2a6231a 100644 --- a/plugins/catalog-backend/src/database/types.ts +++ b/plugins/catalog-backend/src/database/types.ts @@ -153,7 +153,7 @@ export type Database = { matchingGeneration?: number, ): Promise; - entities(tx: unknown, filters?: EntityFilters): Promise; + entities(tx: unknown, filters?: EntityFilters[]): Promise; entityByName( tx: unknown, diff --git a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts index cb161d1ccf..0e62815be5 100644 --- a/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts +++ b/plugins/catalog-backend/src/ingestion/HigherOrderOperations.ts @@ -100,9 +100,9 @@ export class HigherOrderOperations implements HigherOrderOperation { location.id, ); - const entities = await this.entitiesCatalog.entities({ - 'metadata.uid': writtenEntities.map(e => e.entityId), - }); + const entities = await this.entitiesCatalog.entities([ + { 'metadata.uid': writtenEntities.map(e => e.entityId) }, + ]); return { location, entities }; } diff --git a/plugins/catalog-backend/src/service/filterQuery.test.ts b/plugins/catalog-backend/src/service/filterQuery.test.ts new file mode 100644 index 0000000000..b4e205de7b --- /dev/null +++ b/plugins/catalog-backend/src/service/filterQuery.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + translateQueryToEntityFilters, + translateFilterQueryEntryToEntityFilters, +} from './filterQuery'; + +describe('translateQueryToEntityFilters', () => { + it('translates empty query to empty list', () => { + const result = translateQueryToEntityFilters({}); + expect(result).toEqual([]); + }); + + it('supports single-string format', () => { + const result = translateQueryToEntityFilters({ filter: 'a=1' }); + expect(result).toEqual([{ a: ['1'] }]); + }); + + it('supports array-of-strings format', () => { + const result = translateQueryToEntityFilters({ filter: ['a=1', 'b=2'] }); + expect(result).toEqual([{ a: ['1'] }, { b: ['2'] }]); + }); + + it('throws for non-strings', () => { + expect(() => translateQueryToEntityFilters({ filter: [3] })).toThrow( + /string/, + ); + }); +}); + +describe('translateFilterQueryEntryToEntityFilters', () => { + it('runs the happy path', () => { + const result = translateFilterQueryEntryToEntityFilters('a=1,b=2'); + expect(result).toEqual({ a: ['1'], b: ['2'] }); + }); + + it('ignores empty', () => { + const result = translateFilterQueryEntryToEntityFilters('a=1,,b=2,'); + expect(result).toEqual({ a: ['1'], b: ['2'] }); + }); + + it('trims', () => { + const result = translateFilterQueryEntryToEntityFilters(' a = 1 ,, b=2 ,'); + expect(result).toEqual({ a: ['1'], b: ['2'] }); + }); + + it('merges multiple of the same key', () => { + const result = translateFilterQueryEntryToEntityFilters('a=1,a=2,b=3'); + expect(result).toEqual({ a: ['1', '2'], b: ['3'] }); + }); + + it('treats missing equal signs as presence', () => { + const result = translateFilterQueryEntryToEntityFilters('a,b=2'); + expect(result).toEqual({ a: ['*'], b: ['2'] }); + }); + + it('treats empty value as null/absence', () => { + const result = translateFilterQueryEntryToEntityFilters('a=,b=2'); + expect(result).toEqual({ a: [null], b: ['2'] }); + }); +}); diff --git a/plugins/catalog-backend/src/service/filterQuery.ts b/plugins/catalog-backend/src/service/filterQuery.ts new file mode 100644 index 0000000000..23c7c40e8d --- /dev/null +++ b/plugins/catalog-backend/src/service/filterQuery.ts @@ -0,0 +1,72 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/backend-common'; +import { EntityFilters } from '../database'; + +export function translateQueryToEntityFilters( + query: Record, +): EntityFilters[] { + if (!query.filter) { + return []; + } + + const filterStrings = [query.filter].flat(); + + if (filterStrings.some(s => typeof s !== 'string')) { + throw new InputError( + 'Only string type filter query parameters are supported', + ); + } + + return filterStrings + .filter(Boolean) + .map(translateFilterQueryEntryToEntityFilters); +} + +// Parses the value of a filter=a=1,b=2 type query param +export function translateFilterQueryEntryToEntityFilters( + filterString: string, +): EntityFilters { + const filters: Record = {}; + + const addFilter = (key: string, value: string | null) => { + const matchers = key in filters ? filters[key] : (filters[key] = []); + matchers.push(value || null); + }; + + const statements = filterString + .split(',') + .map(s => s.trim()) + .filter(Boolean); + + for (const statement of statements) { + const equalsIndex = statement.indexOf('='); + if (equalsIndex < 0) { + // Check presence, any value + addFilter(statement, '*'); + } else { + const key = statement.substr(0, equalsIndex).trim(); + const value = statement.substr(equalsIndex + 1).trim(); + if (!key) { + throw new InputError('Malformed filter query'); + } + addFilter(key, value); + } + } + + return filters; +} diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index cf843acf3c..03aca11d03 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -76,15 +76,19 @@ describe('createRouter', () => { }); it('parses single and multiple request parameters and passes them down', async () => { - const response = await request(app).get('/entities?a=1&a=&a=3&b=4&c='); + const response = await request(app).get( + '/entities?filter=a=1,a=,a=3,b=4&filter=c=', + ); expect(response.status).toEqual(200); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - a: ['1', null, '3'], - b: ['4'], - c: [null], - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { + a: ['1', null, '3'], + b: ['4'], + }, + { c: [null] }, + ]); }); }); @@ -102,9 +106,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - 'metadata.uid': 'zzz', - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'zzz' }, + ]); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -115,9 +119,9 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-uid/zzz'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - 'metadata.uid': 'zzz', - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'zzz' }, + ]); expect(response.status).toEqual(404); expect(response.text).toMatch(/uid/); }); @@ -138,11 +142,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/k/ns/n'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - kind: 'k', - 'metadata.namespace': 'ns', - 'metadata.name': 'n', - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { + kind: 'k', + 'metadata.namespace': 'ns', + 'metadata.name': 'n', + }, + ]); expect(response.status).toEqual(200); expect(response.body).toEqual(expect.objectContaining(entity)); }); @@ -153,11 +159,13 @@ describe('createRouter', () => { const response = await request(app).get('/entities/by-name/b/d/c'); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - kind: 'b', - 'metadata.namespace': 'd', - 'metadata.name': 'c', - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { + kind: 'b', + 'metadata.namespace': 'd', + 'metadata.name': 'c', + }, + ]); expect(response.status).toEqual(404); expect(response.text).toMatch(/name/); }); @@ -200,9 +208,9 @@ describe('createRouter', () => { { entity, relations: [] }, ]); expect(entitiesCatalog.entities).toHaveBeenCalledTimes(1); - expect(entitiesCatalog.entities).toHaveBeenCalledWith({ - 'metadata.uid': 'u', - }); + expect(entitiesCatalog.entities).toHaveBeenCalledWith([ + { 'metadata.uid': 'u' }, + ]); expect(response.status).toEqual(200); expect(response.body).toEqual(entity); }); diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index aefcca28e2..22005b3ff1 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -14,15 +14,15 @@ * limitations under the License. */ -import { errorHandler, InputError } from '@backstage/backend-common'; +import { errorHandler } from '@backstage/backend-common'; import { locationSpecSchema, repoPathSchema } from '@backstage/catalog-model'; import type { Entity } from '@backstage/catalog-model'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { EntitiesCatalog, LocationsCatalog } from '../catalog'; -import { EntityFilters } from '../database'; import { ConfigGenerator, HigherOrderOperation } from '../ingestion/types'; +import { translateQueryToEntityFilters } from './filterQuery'; import { requireRequestBody, validateRequestBody } from './util'; export interface RouterOptions { @@ -49,7 +49,7 @@ export async function createRouter( if (entitiesCatalog) { router .get('/entities', async (req, res) => { - const filters = translateQueryToEntityFilters(req); + const filters = translateQueryToEntityFilters(req.query); const entities = await entitiesCatalog.entities(filters); res.status(200).send(entities); }) @@ -58,16 +58,18 @@ export async function createRouter( const [result] = await entitiesCatalog.batchAddOrUpdateEntities([ { entity: body as Entity, relations: [] }, ]); - const [entity] = await entitiesCatalog.entities({ - 'metadata.uid': result.entityId, - }); + const [entity] = await entitiesCatalog.entities([ + { 'metadata.uid': result.entityId }, + ]); res.status(200).send(entity); }) .get('/entities/by-uid/:uid', async (req, res) => { const { uid } = req.params; - const entities = await entitiesCatalog.entities({ - 'metadata.uid': uid, - }); + const entities = await entitiesCatalog.entities([ + { + 'metadata.uid': uid, + }, + ]); if (!entities.length) { res.status(404).send(`No entity with uid ${uid}`); } @@ -80,11 +82,13 @@ export async function createRouter( }) .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => { const { kind, namespace, name } = req.params; - const entities = await entitiesCatalog.entities({ - kind: kind, - 'metadata.namespace': namespace, - 'metadata.name': name, - }); + const entities = await entitiesCatalog.entities([ + { + kind: kind, + 'metadata.namespace': namespace, + 'metadata.name': name, + }, + ]); if (!entities.length) { res .status(404) @@ -138,24 +142,3 @@ export async function createRouter( router.use(errorHandler()); return router; } - -function translateQueryToEntityFilters( - request: express.Request, -): EntityFilters { - const filters: Record = {}; - - for (const [key, valueOrValues] of Object.entries(request.query)) { - const values = Array.isArray(valueOrValues) - ? valueOrValues - : [valueOrValues]; - - if (values.some(v => typeof v !== 'string')) { - throw new InputError('Complex query parameters are not supported'); - } - - const matchers = key in filters ? filters[key] : (filters[key] = []); - matchers.push(...(values.map(v => v || null) as (string | null)[])); - } - - return filters; -} diff --git a/plugins/catalog/src/api/CatalogClient.test.ts b/plugins/catalog/src/api/CatalogClient.test.ts index 7c0123317d..927e8dd52c 100644 --- a/plugins/catalog/src/api/CatalogClient.test.ts +++ b/plugins/catalog/src/api/CatalogClient.test.ts @@ -34,7 +34,7 @@ describe('CatalogClient', () => { client = new CatalogClient({ discoveryApi }); }); - describe('getEntiies', () => { + describe('getEntities', () => { const defaultResponse: Entity[] = [ { apiVersion: '1', @@ -71,9 +71,7 @@ describe('CatalogClient', () => { expect.assertions(2); server.use( rest.get(`${mockBaseUrl}/entities`, (req, res, ctx) => { - expect(req.url.searchParams.toString()).toBe( - 'a=1&b=2&b=3&%C3%B6=%3D', - ); + expect(req.url.search).toBe('?filter=a=1,b=2,b=3,%C3%B6=%3D'); return res(ctx.json([])); }), ); diff --git a/plugins/catalog/src/api/CatalogClient.ts b/plugins/catalog/src/api/CatalogClient.ts index 6b10541107..642183f916 100644 --- a/plugins/catalog/src/api/CatalogClient.ts +++ b/plugins/catalog/src/api/CatalogClient.ts @@ -68,17 +68,13 @@ export class CatalogClient implements CatalogApi { ): Promise { let path = `/entities`; if (filter) { - const params = new URLSearchParams(); + const parts: string[] = []; for (const [key, value] of Object.entries(filter)) { - if (Array.isArray(value)) { - for (const v of value) { - params.append(key, v); - } - } else { - params.append(key, value); + for (const v of [value].flat()) { + parts.push(`${encodeURIComponent(key)}=${encodeURIComponent(v)}`); } } - path += `?${params.toString()}`; + path += `?filter=${parts.join(',')}`; } return await this.getRequired(path); diff --git a/plugins/cost-insights/src/components/CostGrowth/CostGrowth.test.tsx b/plugins/cost-insights/src/components/CostGrowth/CostGrowth.test.tsx index 6302d9929f..026b16c74a 100644 --- a/plugins/cost-insights/src/components/CostGrowth/CostGrowth.test.tsx +++ b/plugins/cost-insights/src/components/CostGrowth/CostGrowth.test.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ReactNode } from 'react'; +import React, { PropsWithChildren } from 'react'; import { renderInTestApp } from '@backstage/test-utils'; import CostGrowth from './CostGrowth'; import { @@ -37,21 +37,12 @@ const MockContext = ({ children, currency, engineerCost, -}: { - children: ReactNode; +}: PropsWithChildren<{ currency: Currency; engineerCost: number; -}) => ( - - - {children} - +}>) => ( + + {children} ); diff --git a/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.test.tsx b/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.test.tsx index 7ca4958c8e..ac8561aa91 100644 --- a/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.test.tsx +++ b/plugins/cost-insights/src/components/CostInsightsNavigation/CostInsightsNavigation.test.tsx @@ -18,7 +18,7 @@ import React from 'react'; import { default as HappyFace } from '@material-ui/icons/SentimentSatisfiedAlt'; import { renderInTestApp } from '@backstage/test-utils'; import CostInsightsNavigation from './CostInsightsNavigation'; -import { defaultCurrencies, Metric, Product, Icon } from '../../types'; +import { Product, Icon } from '../../types'; import { MockConfigProvider, MockScrollProvider } from '../../utils/tests'; import { getDefaultNavigationItems } from '../../utils/navigation'; @@ -36,23 +36,9 @@ const mockProducts: Product[] = [ }, ]; -const mockMetrics: Metric[] = [ - { - kind: 'some-metric', - name: 'Some Metric', - default: false, - }, -]; - const renderWrapped = (children: React.ReactNode) => renderInTestApp( - + {children} , ); diff --git a/plugins/cost-insights/src/components/CostInsightsTabs/CostInsightsTabs.test.tsx b/plugins/cost-insights/src/components/CostInsightsTabs/CostInsightsTabs.test.tsx index 0859ad18ce..b75e0ab0d9 100644 --- a/plugins/cost-insights/src/components/CostInsightsTabs/CostInsightsTabs.test.tsx +++ b/plugins/cost-insights/src/components/CostInsightsTabs/CostInsightsTabs.test.tsx @@ -17,14 +17,11 @@ import React from 'react'; import CostInsightsTabs from './CostInsightsTabs'; import UserEvent from '@testing-library/user-event'; -import { Group, defaultCurrencies } from '../../types'; -import { MockFilterProvider, MockConfigProvider } from '../../utils/tests'; -import { LoadingContext } from '../../hooks/useLoading'; +import { Group } from '../../types'; +import { MockFilterProvider, MockLoadingProvider } from '../../utils/tests'; import { renderInTestApp } from '@backstage/test-utils'; -import { mockDefaultState } from '../../utils/mockData'; const mockSetPageFilters = jest.fn(); -const mockLoadingDispatch = jest.fn(); const mockGroups: Group[] = [ { @@ -41,28 +38,9 @@ const mockGroups: Group[] = [ describe('', () => { const renderWrapped = (children: React.ReactNode) => renderInTestApp( - - - - {children} - - - , + + {children} + , ); it('Does NOT display the tabs bar if owner belongs to less than two GROUPS', async () => { diff --git a/plugins/cost-insights/src/components/PeriodSelect/PeriodSelect.test.tsx b/plugins/cost-insights/src/components/PeriodSelect/PeriodSelect.test.tsx index b5381cdd84..3c55285908 100644 --- a/plugins/cost-insights/src/components/PeriodSelect/PeriodSelect.test.tsx +++ b/plugins/cost-insights/src/components/PeriodSelect/PeriodSelect.test.tsx @@ -16,11 +16,10 @@ import React from 'react'; import { getByRole, waitFor } from '@testing-library/react'; +import { renderInTestApp } from '@backstage/test-utils'; import UserEvent from '@testing-library/user-event'; import PeriodSelect, { getDefaultOptions } from './PeriodSelect'; import { Duration, getDefaultPageFilters, Group } from '../../types'; - -import { renderInTestApp } from '@backstage/test-utils'; import { MockBillingDateProvider } from '../../utils/tests'; const DefaultPageFilters = getDefaultPageFilters([{ id: 'tools' }] as Group[]); diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx index 1ca1b48c6c..bae4eef591 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.test.tsx @@ -15,50 +15,27 @@ */ import React from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; import ProductInsightsCard from './ProductInsightsCard'; +import { CostInsightsApi } from '../../api'; import { createMockEntity, createMockProductCost, + mockDefaultLoadingState, MockComputeEngine, - mockDefaultState, + MockProductFilters, } from '../../utils/mockData'; import { - ApiProvider, - ApiRegistry, - IdentityApi, - identityApiRef, -} from '@backstage/core'; -import { CostInsightsApi, costInsightsApiRef } from '../../api'; -import { renderInTestApp } from '@backstage/test-utils'; -import { GroupsContext } from '../../hooks/useGroups'; -import { LoadingContext } from '../../hooks/useLoading'; -import { - defaultCurrencies, - Duration, - findAlways, - Product, - ProductCost, -} from '../../types'; -import { + MockCostInsightsApiProvider, MockBillingDateProvider, MockConfigProvider, MockCurrencyProvider, MockFilterProvider, + MockGroupsProvider, MockScrollProvider, + MockLoadingProvider, } from '../../utils/tests'; - -const mockLoadingDispatch = jest.fn(); -const mockSetPageFilters = jest.fn(); -const mockSetProductFilters = jest.fn(); -const mockSetCurrency = jest.fn(); -const engineers = findAlways(defaultCurrencies, c => c.kind === null); - -const identityApi: Partial = { - getProfile: () => ({ - email: 'test-email@example.com', - displayName: 'User 1', - }), -}; +import { Duration, Product, ProductCost, ProductPeriod } from '../../types'; const costInsightsApi = ( productCost: ProductCost, @@ -67,13 +44,6 @@ const costInsightsApi = ( Promise.resolve(productCost) as Promise, }); -const getApis = (productCost: ProductCost) => { - return ApiRegistry.from([ - [identityApiRef, identityApi], - [costInsightsApiRef, costInsightsApi(productCost)], - ]); -}; - const mockProductCost = createMockProductCost(() => ({ entities: [], aggregation: [3000, 4000], @@ -89,42 +59,28 @@ const renderProductInsightsCardInTestApp = async ( duration: Duration, ) => await renderInTestApp( - - - - - + + + + + ({ + ...p, + duration: duration, + }))} > - + - - + + - , + , ); describe('', () => { diff --git a/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertCard.test.tsx b/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertCard.test.tsx index dd70433003..97775258c9 100644 --- a/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertCard.test.tsx +++ b/plugins/cost-insights/src/components/ProjectGrowthAlertCard/ProjectGrowthAlertCard.test.tsx @@ -46,15 +46,9 @@ describe('', () => { ); const title = new RegExp(`Project growth for ${MockProject}`); const rendered = await renderInTestApp( - + - + , @@ -71,13 +65,7 @@ describe('', () => { const subheader = new RegExp('1 product'); const title = new RegExp(`Project growth for ${MockProject}`); const rendered = await renderInTestApp( - + ', () => { let Component: React.ReactNode; beforeEach(() => { Component = () => ( - + ); diff --git a/plugins/cost-insights/src/components/ResourceGrowthBarChartLegend/ResourceGrowthBarChartLegend.test.tsx b/plugins/cost-insights/src/components/ResourceGrowthBarChartLegend/ResourceGrowthBarChartLegend.test.tsx index ecc601a0b7..e3c2506296 100644 --- a/plugins/cost-insights/src/components/ResourceGrowthBarChartLegend/ResourceGrowthBarChartLegend.test.tsx +++ b/plugins/cost-insights/src/components/ResourceGrowthBarChartLegend/ResourceGrowthBarChartLegend.test.tsx @@ -14,31 +14,17 @@ * limitations under the License. */ -import React, { ReactNode } from 'react'; +import React, { PropsWithChildren } from 'react'; import { renderInTestApp } from '@backstage/test-utils'; import ResourceGrowthBarChartLegend from './ResourceGrowthBarChartLegend'; -import { Currency, defaultCurrencies, Duration, findAlways } from '../../types'; +import { defaultCurrencies, Duration, findAlways } from '../../types'; import { MockConfigProvider, MockCurrencyProvider } from '../../utils/tests'; const engineers = findAlways(defaultCurrencies, c => c.kind === null); -const MockContext = ({ - children, - currency, -}: { - children: ReactNode; - currency: Currency; -}) => ( - - - {children} - +const MockContext = ({ children }: PropsWithChildren<{}>) => ( + + {children} ); @@ -52,7 +38,7 @@ describe('', () => { ({ ratio, amount, costText, engineerTest }) => { it(`Should display the correct cost and engineer text for ${ratio} percent change`, async () => { const rendered = await renderInTestApp( - + = ({ dispatch({ [DefaultLoadingAction.UserGroups]: isLoading }), }); -type GroupsContextProps = { +export type GroupsContextProps = { groups: Group[]; }; diff --git a/plugins/cost-insights/src/index.ts b/plugins/cost-insights/src/index.ts index ba912d2316..71cc66dba6 100644 --- a/plugins/cost-insights/src/index.ts +++ b/plugins/cost-insights/src/index.ts @@ -19,3 +19,4 @@ export * from './api'; export * from './components'; export { useCurrency } from './hooks'; export * from './types'; +export * from './utils/tests'; diff --git a/plugins/cost-insights/src/utils/mockData.ts b/plugins/cost-insights/src/utils/mockData.ts index a25f0dad20..11c70eee25 100644 --- a/plugins/cost-insights/src/utils/mockData.ts +++ b/plugins/cost-insights/src/utils/mockData.ts @@ -19,7 +19,7 @@ import { Duration, Entity, findAlways, - getDefaultState, + getDefaultState as getDefaultLoadingState, Product, ProductCost, ProductFilters, @@ -154,7 +154,9 @@ export const MockLoadingActions = ([ DefaultLoadingAction.CostInsightsPage, ] as string[]).concat(MockProducts.map(product => product.kind)); -export const mockDefaultState = getDefaultState(MockLoadingActions); +export const mockDefaultLoadingState = getDefaultLoadingState( + MockLoadingActions, +); export const MockComputeEngine = findAlways( MockProducts, diff --git a/plugins/cost-insights/src/utils/tests.tsx b/plugins/cost-insights/src/utils/tests.tsx index fbf665c131..db47bd72bd 100644 --- a/plugins/cost-insights/src/utils/tests.tsx +++ b/plugins/cost-insights/src/utils/tests.tsx @@ -13,96 +13,221 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { Dispatch, PropsWithChildren, SetStateAction } from 'react'; + +import React, { PropsWithChildren } from 'react'; +import { costInsightsApiRef, CostInsightsApi } from '../api'; import { - Duration, - getDefaultPageFilters, - Group, - Maybe, - PageFilters, - ProductFilters, - ProductPeriod, -} from '../types'; -import { FilterContext } from '../hooks/useFilters'; + ApiProvider, + ApiRegistry, + IdentityApi, + identityApiRef, +} from '@backstage/core'; +import { LoadingContext, LoadingContextProps } from '../hooks/useLoading'; +import { GroupsContext, GroupsContextProps } from '../hooks/useGroups'; +import { FilterContext, FilterContextProps } from '../hooks/useFilters'; import { ConfigContext, ConfigContextProps } from '../hooks/useConfig'; import { CurrencyContext, CurrencyContextProps } from '../hooks/useCurrency'; -import { ScrollContext } from '../hooks/useScroll'; import { BillingDateContext, BillingDateContextProps, } from '../hooks/useLastCompleteBillingDate'; -import { MockProductFilters } from './mockData'; +import { ScrollContext, ScrollContextProps } from '../hooks/useScroll'; +import { Duration, Group } from '../types'; + +/* + Mock Providers and types are exposed publicly to allow users to test custom implementations + such as alerts, which may require context. + + Utility functions such as getDefaultPageFilters, etc. are intentionally + omitted as we do not want to expose explictly or implicitly internal implementations + that may be subject to change. + + Each Mock Provider provides minimal defaults which can be overridden, allowing users to define + context props only when necessary. +*/ + +type PartialPropsWithChildren = PropsWithChildren>; export const MockGroups: Group[] = [{ id: 'tech' }, { id: 'mock-group' }]; -type MockFilterProviderProps = { - setPageFilters: Dispatch>>; - setProductFilters: Dispatch>>; - duration?: Duration; -}; +export type MockFilterProviderProps = PartialPropsWithChildren< + FilterContextProps +>; export const MockFilterProvider = ({ - setPageFilters, - setProductFilters, children, - duration = Duration.P1M, -}: PropsWithChildren) => { - const pageFilters = getDefaultPageFilters(MockGroups); + ...context +}: MockFilterProviderProps) => { + const defaultContext: FilterContextProps = { + pageFilters: { + group: 'tech', + project: null, + duration: Duration.P90D, + metric: null, + }, + productFilters: [], + setPageFilters: jest.fn(), + setProductFilters: jest.fn(), + }; return ( - ({ - ...period, - duration: duration, - })), - setPageFilters: setPageFilters, - setProductFilters: setProductFilters, - }} - > + {children} ); }; -export const MockConfigProvider = ({ - metrics, - products, - icons, - engineerCost, - currencies, +export type MockLoadingProviderProps = PartialPropsWithChildren< + LoadingContextProps +>; + +export const MockLoadingProvider = ({ children, -}: PropsWithChildren) => ( - - {children} - -); + ...context +}: MockLoadingProviderProps) => { + const defaultContext: LoadingContextProps = { + state: {}, + actions: [], + dispatch: jest.fn(), + }; + return ( + + {children} + + ); +}; + +export type MockConfigProviderProps = PartialPropsWithChildren< + ConfigContextProps +>; + +export const MockConfigProvider = ({ + children, + ...context +}: MockConfigProviderProps) => { + const defaultContext: ConfigContextProps = { + metrics: [], + products: [], + icons: [], + engineerCost: 0, + currencies: [], + }; + return ( + + {children} + + ); +}; + +export type MockCurrencyProviderProps = PartialPropsWithChildren< + CurrencyContextProps +>; export const MockCurrencyProvider = ({ - currency, - setCurrency, children, -}: PropsWithChildren) => ( - - {children} - -); + ...context +}: MockCurrencyProviderProps) => { + const defaultContext: CurrencyContextProps = { + currency: { + kind: null, + label: 'Engineers 🛠', + unit: 'engineer', + }, + setCurrency: jest.fn(), + }; + return ( + + {children} + + ); +}; + +export type MockBillingDateProviderProps = PartialPropsWithChildren< + BillingDateContextProps +>; export const MockBillingDateProvider = ({ - lastCompleteBillingDate, children, -}: PropsWithChildren) => ( - - {children} - -); + ...context +}: MockBillingDateProviderProps) => { + const defaultContext: BillingDateContextProps = { + lastCompleteBillingDate: '2020-10-01', + }; + return ( + + {children} + + ); +}; -export const MockScrollProvider = ({ children }: PropsWithChildren<{}>) => ( - - {children} - -); +export type MockScrollProviderProps = PropsWithChildren<{}>; + +export const MockScrollProvider = ({ children }: MockScrollProviderProps) => { + const defaultContext: ScrollContextProps = { + scrollTo: null, + setScrollTo: jest.fn(), + }; + return ( + + {children} + + ); +}; + +export type MockGroupsProviderProps = PartialPropsWithChildren< + GroupsContextProps +>; + +export const MockGroupsProvider = ({ + children, + ...context +}: MockGroupsProviderProps) => { + const defaultContext: GroupsContextProps = { + groups: [], + }; + return ( + + {children} + + ); +}; + +export type MockCostInsightsApiProviderProps = PartialPropsWithChildren<{ + identityApi: Partial; + costInsightsApi: Partial; +}>; + +export const MockCostInsightsApiProvider = ({ + children, + ...context +}: MockCostInsightsApiProviderProps) => { + const defaultIdentityApi: IdentityApi = { + getProfile: jest.fn(), + getIdToken: jest.fn(), + getUserId: jest.fn(), + signOut: jest.fn(), + }; + + const defaultCostInsightsApi: CostInsightsApi = { + getAlerts: jest.fn(), + getDailyMetricData: jest.fn(), + getGroupDailyCost: jest.fn(), + getGroupProjects: jest.fn(), + getLastCompleteBillingDate: jest.fn(), + getProductInsights: jest.fn(), + getProjectDailyCost: jest.fn(), + getUserGroups: jest.fn(), + }; + + // TODO: defaultConfigApiRef: ConfigApiRef + + const defaultContext = ApiRegistry.from([ + [identityApiRef, { ...defaultIdentityApi, ...context.identityApi }], + [ + costInsightsApiRef, + { ...defaultCostInsightsApi, ...context.costInsightsApi }, + ], + // [configApiRef, { ...defaultConfigApiRef, ...context.configApiRef }] + ]); + + return {children}; +}; diff --git a/plugins/lighthouse/src/components/CreateAudit/index.tsx b/plugins/lighthouse/src/components/CreateAudit/index.tsx index 372992a718..55800d79ec 100644 --- a/plugins/lighthouse/src/components/CreateAudit/index.tsx +++ b/plugins/lighthouse/src/components/CreateAudit/index.tsx @@ -42,6 +42,9 @@ import LighthouseSupportButton from '../SupportButton'; const useStyles = makeStyles(theme => ({ input: { minWidth: 300, + [theme.breakpoints.down('xs')]: { + minWidth: '100%', + }, }, buttonList: { marginLeft: theme.spacing(-1), @@ -49,6 +52,14 @@ const useStyles = makeStyles(theme => ({ '& > *': { margin: theme.spacing(1), }, + [theme.breakpoints.down('xs')]: { + marginLeft: 0, + marginRight: 0, + flexDirection: 'column', + '& > *': { + width: '100%', + }, + }, }, })); diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 0d05156791..ec197dbc55 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -36,7 +36,7 @@ "express": "^4.17.1", "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", - "git-url-parse": "^11.3.0", + "git-url-parse": "^11.4.0", "globby": "^11.0.0", "helmet": "^4.0.0", "jsonschema": "^1.2.6", diff --git a/plugins/techdocs-backend/package.json b/plugins/techdocs-backend/package.json index 60c7c8f084..e3878e95b5 100644 --- a/plugins/techdocs-backend/package.json +++ b/plugins/techdocs-backend/package.json @@ -30,7 +30,7 @@ "express": "^4.17.1", "express-promise-router": "^3.0.3", "fs-extra": "^9.0.1", - "git-url-parse": "^11.3.0", + "git-url-parse": "^11.4.0", "knex": "^0.21.1", "nodegit": "^0.27.0", "cross-fetch": "^3.0.6", diff --git a/plugins/techdocs/src/reader/components/Reader.tsx b/plugins/techdocs/src/reader/components/Reader.tsx index 01299b8fe6..785c6f1c6b 100644 --- a/plugins/techdocs/src/reader/components/Reader.tsx +++ b/plugins/techdocs/src/reader/components/Reader.tsx @@ -30,6 +30,7 @@ import transformer, { rewriteDocLinks, addLinkClickListener, removeMkdocsHeader, + simplifyMkdocsFooter, modifyCss, onCssReady, sanitizeDOM, @@ -81,6 +82,7 @@ export const Reader = ({ entityId, onReady }: Props) => { }, }), removeMkdocsHeader(), + simplifyMkdocsFooter(), injectCss({ css: ` body { diff --git a/plugins/techdocs/src/reader/transformers/index.ts b/plugins/techdocs/src/reader/transformers/index.ts index ddbaadd071..0bab085abe 100644 --- a/plugins/techdocs/src/reader/transformers/index.ts +++ b/plugins/techdocs/src/reader/transformers/index.ts @@ -18,6 +18,7 @@ export * from './addBaseUrl'; export * from './rewriteDocLinks'; export * from './addLinkClickListener'; export * from './removeMkdocsHeader'; +export * from './simplifyMkdocsFooter'; export * from './modifyCss'; export * from './onCssReady'; export * from './sanitizeDOM'; diff --git a/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.test.ts b/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.test.ts new file mode 100644 index 0000000000..5af62a43c5 --- /dev/null +++ b/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.test.ts @@ -0,0 +1,38 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createTestShadowDom, FIXTURES } from '../../test-utils'; +import { simplifyMkdocsFooter } from '.'; + +describe('simplifyMkdocsFooter', () => { + it('does not remove mkdocs copyright', () => { + const shadowDom = createTestShadowDom(FIXTURES.FIXTURE_STANDARD_PAGE, { + preTransformers: [], + postTransformers: [], + }); + + expect(shadowDom.querySelector('.md-footer-copyright')).toBeTruthy(); + }); + + it('does remove mkdocs copyright', () => { + const shadowDom = createTestShadowDom(FIXTURES.FIXTURE_STANDARD_PAGE, { + preTransformers: [simplifyMkdocsFooter()], + postTransformers: [], + }); + + expect(shadowDom.querySelector('.md-footer-copyright')).toBeFalsy(); + }); +}); diff --git a/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.ts b/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.ts new file mode 100644 index 0000000000..0d0745724d --- /dev/null +++ b/plugins/techdocs/src/reader/transformers/simplifyMkdocsFooter.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Transformer } from './index'; + +export const simplifyMkdocsFooter = (): Transformer => { + return dom => { + // Remove mkdocs copyright + dom.querySelector('.md-footer-copyright')?.remove(); + + return dom; + }; +}; diff --git a/yarn.lock b/yarn.lock index 84334952e8..3e89bd93e7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12433,10 +12433,10 @@ git-url-parse@^11.1.2: dependencies: git-up "^4.0.0" -git-url-parse@^11.3.0: - version "11.3.0" - resolved "https://registry.npmjs.org/git-url-parse/-/git-url-parse-11.3.0.tgz#1515b4574c4eb2efda7d25cc50b29ce8beaefaae" - integrity sha512-i3XNa8IKmqnUqWBcdWBjOcnyZYfN3C1WRvnKI6ouFWwsXCZEnlgbwbm55ZpJ3OJMhfEP/ryFhqW8bBhej3C5Ug== +git-url-parse@^11.4.0: + version "11.4.0" + resolved "https://registry.npmjs.org/git-url-parse/-/git-url-parse-11.4.0.tgz#f2bb1f2b00f05552540e95a62e31399a639a6aa6" + integrity sha512-KlIa5jvMYLjXMQXkqpFzobsyD/V2K5DRHl5OAf+6oDFPlPLxrGDVQlIdI63c4/Kt6kai4kALENSALlzTGST3GQ== dependencies: git-up "^4.0.0"