Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[IM] add screenshot detail view and image endpoint #48149

5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/integrations_manager/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export interface RegistryListItem {
title?: string;
}

export interface ScreenshotItem {
src: string;
}

// from /package/{name}
// https://github.com/elastic/integrations-registry/blob/master/docs/api/package.json
export type ServiceName = 'kibana' | 'elasticsearch' | 'filebeat' | 'metricbeat';
Expand Down Expand Up @@ -67,6 +71,7 @@ export interface RegistryPackage {
icon: string;
requirement: RequirementsByServiceName;
title?: string;
screenshots?: ScreenshotItem[];
}

// Managers public HTTP response types
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { generatePath } from 'react-router-dom';
import { PLUGIN } from '../../common/constants';
import { API_ROOT } from '../../common/routes';
import { patterns } from '../routes';
import { useCore } from '.';
import { DetailViewPanelName } from '..';
Expand All @@ -31,6 +32,7 @@ function appRoot(path: string) {
export function useLinks() {
return {
toAssets: (path: string) => addBasePath(`/plugins/${PLUGIN.ID}/assets/${path}`),
toImage: (path: string) => addBasePath(`${API_ROOT}${path}`),
toListView: () => appRoot(patterns.LIST_VIEW),
toDetailView: ({ name, version, panel }: DetailParams) => {
// panel is optional, but `generatePath` won't accept `path: undefined`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ export function Content(props: ContentProps) {
`
: LeftColumn;

// fixes IE11 problem with nested flex items
const ContentFlexGroup = styled(EuiFlexGroup)`
flex: 0 0 auto !important;
`;
return (
<EuiFlexGroup>
<ContentFlexGroup>
<SideNavColumn>
<SideNavLinks name={name} version={version} active={panel || DEFAULT_PANEL} />
</SideNavColumn>
Expand All @@ -48,7 +52,7 @@ export function Content(props: ContentProps) {
<RightColumn>
<RightColumnContent {...props} />
</RightColumn>
</EuiFlexGroup>
</ContentFlexGroup>
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,21 @@
import React, { Fragment } from 'react';
import { EuiSpacer, EuiText, EuiTitle } from '@elastic/eui';
import { IntegrationInfo } from '../../../common/types';
import { Screenshots } from './screenshots';

export function OverviewPanel(props: IntegrationInfo) {
const { description } = props;
const { description, screenshots } = props;
return (
<Fragment>
<EuiTitle size="xs">
<span>About</span>
<EuiTitle size="s">
<h3>About</h3>
</EuiTitle>
<EuiText>
<p>{description}</p>
<p>Still need a) longer descriptions b) component to show/hide</p>
</EuiText>
<EuiSpacer size="xl" />
<EuiTitle size="xs">
<span>Screenshots</span>
</EuiTitle>
<EuiText>
<p>Where are we getting these images?</p>
</EuiText>
{screenshots && <Screenshots images={screenshots} />}
</Fragment>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import React, { Fragment } from 'react';
import { EuiSpacer, EuiText, EuiTitle, EuiImage, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import styled from 'styled-components';
import { ScreenshotItem } from '../../../common/types';
import { useLinks, useCore } from '../../hooks';

interface ScreenshotProps {
images: ScreenshotItem[];
}

export function Screenshots(props: ScreenshotProps) {
neptunian marked this conversation as resolved.
Show resolved Hide resolved
const { theme } = useCore();
const { toImage } = useLinks();
const { images } = props;

// for now, just get first image
const src = toImage(images[0].src);

const horizontalPadding: number = parseInt(theme.eui.paddingSizes.xl, 10) * 2;
const bottomPadding: number = parseInt(theme.eui.paddingSizes.xl, 10) * 1.75;

const ScreenshotsContainer = styled(EuiFlexGroup)`
background: linear-gradient(360deg, rgba(0, 0, 0, 0.2) 0%, rgba(0, 0, 0, 0) 100%),
${theme.eui.euiColorPrimary};
padding: ${theme.eui.paddingSizes.xl} ${horizontalPadding}px ${bottomPadding}px;
flex: 0 0 auto;
border-radius: ${theme.eui.euiBorderRadius};
`;
// fixes ie11 problems with nested flex items
const NestedEuiFlexItem = styled(EuiFlexItem)`
flex: 0 0 auto !important;
`;
return (
<Fragment>
<EuiTitle size="s">
<h3>Screenshots</h3>
</EuiTitle>
<EuiSpacer size="m" />
<ScreenshotsContainer gutterSize="none" direction="column" alignItems="center">
<NestedEuiFlexItem>
<EuiText color="ghost" aria-label="screenschot image caption">
We need image descriptions to be returned in the response
</EuiText>
<EuiSpacer />
</NestedEuiFlexItem>
<NestedEuiFlexItem>
<EuiImage url={src} alt="screenhot image preview" size="l" allowFullScreen />
</NestedEuiFlexItem>
</ScreenshotsContainer>
</Fragment>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ export async function getIntegrationInfo(options: {
return createInstallableFrom(updated, savedObject);
}

export const getImage = async (options: Registry.ImageRequestParams) =>
Registry.fetchImage(options);

export async function getInstallationObject(options: {
savedObjectsClient: SavedObjectsClientContract;
pkgkey: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import {
getCategories,
getClusterAccessor,
getIntegrationInfo,
getImage,
getIntegrations,
installIntegration,
removeInstallation,
} from './index';
import { ImageRequestParams } from '../registry';

interface Extra extends ResponseToolkit {
context: PluginContext;
Expand All @@ -31,6 +33,10 @@ interface PackageRequest extends Request {
};
}

interface ImageRequest extends Request {
params: Request['params'] & ImageRequestParams;
}

interface InstallAssetRequest extends Request {
params: AssetRequestParams;
}
Expand Down Expand Up @@ -65,6 +71,14 @@ export async function handleGetInfo(req: PackageRequest, extra: Extra) {
return integrationInfo;
}

export const handleGetImage = async (req: ImageRequest, extra: Extra) => {
const response = await getImage(req.params);
const newResponse = extra.response(response.body);
// set the content type from the registry response
newResponse.header('Content-Type', response.headers.get('content-type') || '');
return newResponse;
};

export async function handleRequestInstall(req: InstallAssetRequest, extra: Extra) {
const { pkgkey, asset } = req.params;
if (!asset) throw new Error('Unhandled empty/default asset case');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { URL } from 'url';
import { Response } from 'node-fetch';
import {
AssetsGroupedByServiceByType,
AssetParts,
Expand All @@ -15,7 +16,7 @@ import {
} from '../../common/types';
import { cacheGet, cacheSet } from './cache';
import { ArchiveEntry, untarBuffer } from './extract';
import { fetchUrl, getResponseStream } from './requests';
import { fetchUrl, getResponseStream, getResponse } from './requests';
import { streamToBuffer } from './streams';
import { integrationsManagerConfigStore } from '../config';

Expand All @@ -25,6 +26,11 @@ export interface SearchParams {
category?: CategoryId;
}

export interface ImageRequestParams {
pkgkey: string;
imgPath: string;
}

export async function fetchList(params?: SearchParams): Promise<RegistryList> {
const { registryUrl } = integrationsManagerConfigStore.getConfig();
const url = new URL(`${registryUrl}/search`);
Expand All @@ -40,6 +46,12 @@ export async function fetchInfo(key: string): Promise<RegistryPackage> {
return fetchUrl(`${registryUrl}/package/${key}`).then(JSON.parse);
}

export async function fetchImage(params: ImageRequestParams): Promise<Response> {
const { registryUrl } = integrationsManagerConfigStore.getConfig();
const { pkgkey, imgPath } = params;
return getResponse(`${registryUrl}/package/${pkgkey}/img/${imgPath}`);
}

export async function fetchCategories(): Promise<CategorySummaryList> {
const { registryUrl } = integrationsManagerConfigStore.getConfig();
return fetchUrl(`${registryUrl}/categories`).then(JSON.parse);
Expand Down
8 changes: 8 additions & 0 deletions x-pack/legacy/plugins/integrations_manager/server/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { ServerRoute } from '../common/types';
import * as CommonRoutes from '../common/routes';
import * as Integrations from './integrations/handlers';

const API_IMG_PATTERN = `${CommonRoutes.API_ROOT}/package/{pkgkey}/img/{imgPath*}`;

// Manager public API paths
export const routes: ServerRoute[] = [
{
Expand All @@ -22,6 +24,12 @@ export const routes: ServerRoute[] = [
options: { tags: [`access:${PLUGIN.ID}`], json: { space: 2 } },
handler: Integrations.handleGetList,
},
{
method: 'GET',
path: API_IMG_PATTERN,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I might even putting it inline

Suggested change
path: API_IMG_PATTERN,
path: `${CommonRoutes.API_ROOT}/package/{pkgkey}/img/{imgPath*}`,

but not a big deal to define as API_IMG_PATTERN or declare inline

Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use here the direct path to the screenshot from the manifest? An example can be found here: http://integrations-registry.app.elstc.co/package/auditd-2.0.4

Copy link
Contributor Author

@neptunian neptunian Oct 17, 2019

Choose a reason for hiding this comment

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

@ruflin are you saying to just link directly to the registry like: http://integrations-registry.app.elstc.co/package/auditd-2.0.4/img/screenshots/auditbeat-file-integrity-dashboard.png? Otherwise, this route handles both the icon and screenshot images, which have slightly different paths, using the integrations manager api.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think I got what I missed here. These are the routes for the internal calls. What I worry is that we for example have /img hardcoded, so if we decide to move images under media, things break. That is the reason manifest we always have the full relative paths, so images can be put where the package creator prefers to put them.

But lets ignore it for now and move forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruflin I see. Would you have used the file extension (svg, png) for the route instead to differentiate it from other assets?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for catching that @ruflin.

@neptunian let's merge this as-is then do a follow-up where we replace the specific image route, with a new route (alter info route) that is essentially a pass-through/proxy to the registry.

export const API_FILE_PATTERN = `${API_ROOT}/package/{pkgkey}/{pathToFile?}`;

then do a similar process to what you do now with the image where we call the registry and return the response & appropriate headers

options: { tags: [`access:${PLUGIN.ID}`], json: { space: 2 } },
handler: Integrations.handleGetImage,
},
{
method: 'GET',
path: CommonRoutes.API_INFO_PATTERN,
Expand Down
61 changes: 61 additions & 0 deletions x-pack/test/integrations_manager_api_integration/apis/image.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import ServerMock from 'mock-http-server';
import { FtrProviderContext } from '../../api_integration/ftr_provider_context';

export default function({ getService }: FtrProviderContext) {
describe('images', () => {
const server = new ServerMock({ host: 'localhost', port: 6666 });
beforeEach(() => {
server.start(() => {});
});
afterEach(() => {
server.stop(() => {});
});
it('fetches a png screenshot image from the registry', async () => {
server.on({
method: 'GET',
path: '/package/auditd-2.0.4/img/screenshots/auditbeat-file-integrity-dashboard.png',
reply: {
headers: { 'content-type': 'image/png' },
},
});

const supertest = getService('supertest');
const fetchImage = async () => {
neptunian marked this conversation as resolved.
Show resolved Hide resolved
await supertest
.get(
'/api/integrations_manager/package/auditd-2.0.4/img/screenshots/auditbeat-file-integrity-dashboard.png'
)
.set('kbn-xsrf', 'xxx')
.expect('Content-Type', 'image/png')
.expect(200);
};
await fetchImage();
});

it('fetches an icon image from the registry', async () => {
server.on({
method: 'GET',
path: '/package/auditd-2.0.4/img/icon.svg',
reply: {
headers: { 'content-type': 'image/svg' },
},
});

const supertest = getService('supertest');
const fetchImage = async () => {
await supertest
.get('/api/integrations_manager/package/auditd-2.0.4/img/icon.svg')
.set('kbn-xsrf', 'xxx')
.expect('Content-Type', 'image/svg')
.expect(200);
};
await fetchImage();
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ export default function ({ loadTestFile }) {
describe('Integrations Manager Endpoints', function () {
this.tags('ciGroup7');
loadTestFile(require.resolve('./list'));
loadTestFile(require.resolve('./image'));
});
}