[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship w…

…ith spec (#84946) ## Summary * Create a [TS type for `package-spec`](https://github.com/elastic/kibana/blob/09cce235de55f898043e969a234464e62473cba9/x-pack/plugins/fleet/common/types/models/package_spec.ts) to differentiate Registry shape from spec shape * Clarify EPR types relationship with package spec. i.e. what required properties it makes optional, what properties it adds, which property types it changes, etc. https://github.com/elastic/kibana/blob/5f2c4a5547fc9a70b2696ca34c7caff64f3ed674/x-pack/plugins/fleet/common/types/models/epm.ts#L71-L80 * Updated `manifest.yml` in `apache_invalid_manifest_missing_field_0.1.4.zip`. Previously it was missing the `type` property, but that's optional [according to package-spec](https://github.com/elastic/package-spec/blob/3b4d8207554dde6ab9646567dd39b747a2ce167c/versions/1/manifest.spec.yml#L241-L248). Removed the required `format_version` and re-zipped. <details><summary>screenshots show errors if catches in a mock PackageInfo object</summary> using `requirement`; not `conditions` <img width="1084" alt="Screen Shot 2020-12-02 at 12 07 15 PM" src="https://user-images.githubusercontent.com/57655/101083986-4b915f80-357b-11eb-84c6-14605e1ea3d8.png"> using `versions`; not `version` <img width="1395" alt="Screen Shot 2020-12-02 at 12 08 13 PM" src="https://user-images.githubusercontent.com/57655/101083988-4c29f600-357b-11eb-9ff2-2b73e313c130.png"> including `elasticsearch` when spec says it' not valid yet <img width="1589" alt="Screen Shot 2020-12-02 at 12 08 36 PM" src="https://user-images.githubusercontent.com/57655/101083990-4c29f600-357b-11eb-8250-8926f7189af8.png"> </details> <details><summary>screenshots showing editor autocomplete for registry response values like <code>categories</code></summary> <img width="422" alt="Screen Shot 2020-12-02 at 12 25 11 PM" src="https://user-images.githubusercontent.com/57655/101083994-4cc28c80-357b-11eb-9013-ae208f7c311a.png"> <img width="345" alt="Screen Shot 2020-12-02 at 12 25 01 PM" src="https://user-images.githubusercontent.com/57655/101083991-4c29f600-357b-11eb-9468-bbb9d27513b1.png"> </details> <details><summary>screenshot showing type check preventing adding properties which aren't explicitly listed as "additions"</summary> <img width="1295" alt="Screen Shot 2020-12-03 at 11 38 43 AM" src="https://user-images.githubusercontent.com/57655/101083997-4cc28c80-357b-11eb-83b9-206b85521f11.png"> </details> ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
elastic · Dec 7, 2020 · ef7367d · ef7367d
1 parent e476baf
commit ef7367d
Show file tree

Hide file tree

Showing 15 changed files with 189 additions and 96 deletions.
diff --git a/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts b/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts
@@ -3,7 +3,6 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { installationStatuses } from '../constants';
 import { PackageInfo } from '../types';
 import { packageToPackagePolicy, packageToPackagePolicyInputs } from './package_to_package_policy';
 
@@ -14,9 +13,9 @@ describe('Fleet - packageToPackagePolicy', () => {
     version: '0.0.0',
     latestVersion: '0.0.0',
     description: 'description',
-    type: 'mock',
+    type: 'integration',
     categories: [],
-    requirement: { kibana: { versions: '' }, elasticsearch: { versions: '' } },
+    conditions: { kibana: { version: '' } },
     format_version: '',
     download: '',
     path: '',
@@ -29,7 +28,11 @@ describe('Fleet - packageToPackagePolicy', () => {
         map: [],
       },
     },
-    status: installationStatuses.NotInstalled,
+    status: 'not_installed',
+    release: 'experimental',
+    owner: {
+      github: 'elastic/fleet',
+    },
   };
 
   describe('packageToPackagePolicyInputs', () => {

diff --git a/x-pack/plugins/fleet/common/types/models/epm.ts b/x-pack/plugins/fleet/common/types/models/epm.ts
@@ -15,6 +15,7 @@ import {
   requiredPackages,
 } from '../../constants';
 import { ValueOf } from '../../types';
+import { PackageSpecManifest, PackageSpecScreenshot } from './package_spec';
 
 export type InstallationStatus = typeof installationStatuses;
 
@@ -67,63 +68,63 @@ export enum ElasticsearchAssetType {
 
 export type DataType = typeof dataTypes;
 
-export type RegistryRelease = 'ga' | 'beta' | 'experimental';
+export type InstallablePackage = RegistryPackage | ArchivePackage;
 
-// Fields common to packages that come from direct upload and the registry
-export interface InstallablePackage {
-  name: string;
-  title?: string;
-  version: string;
-  release?: RegistryRelease;
-  readme?: string;
-  description: string;
-  type: string;
-  categories: string[];
-  screenshots?: RegistryImage[];
-  icons?: RegistryImage[];
-  assets?: string[];
-  internal?: boolean;
-  format_version: string;
-  data_streams?: RegistryDataStream[];
-  policy_templates?: RegistryPolicyTemplate[];
-}
+export type ArchivePackage = PackageSpecManifest &
+  // should an uploaded package be able to specify `internal`?
+  Pick<RegistryPackage, 'readme' | 'assets' | 'data_streams' | 'internal'>;
 
-// Uploaded package archives don't have extra fields
-// Linter complaint disabled because this extra type is meant for better code readability
-// eslint-disable-next-line @typescript-eslint/no-empty-interface
-export interface ArchivePackage extends InstallablePackage {}
+export type RegistryPackage = PackageSpecManifest &
+  Partial<RegistryOverridesToOptional> &
+  RegistryAdditionalProperties &
+  RegistryOverridePropertyValue;
 
 // Registry packages do have extra fields.
 // cf. type Package struct at https://github.com/elastic/package-registry/blob/master/util/package.go
-export interface RegistryPackage extends InstallablePackage {
-  requirement: RequirementsByServiceName;
+type RegistryOverridesToOptional = Pick<PackageSpecManifest, 'title' | 'release'>;
+
+// our current types have `download`, & `path` as required but they're are optional (have `omitempty`) according to
+// https://github.com/elastic/package-registry/blob/master/util/package.go#L57
+// & https://github.com/elastic/package-registry/blob/master/util/package.go#L80-L81
+// However, they are always present in every registry response I checked. Chose to keep types unchanged for now
+// and confirm with Registry if they are really optional. Can update types and ~4 places in code later if neccessary
+interface RegistryAdditionalProperties {
+  assets?: string[];
   download: string;
   path: string;
+  readme?: string;
+  internal?: boolean; // Registry addition[0] and EPM uses it[1] [0]: https://github.com/elastic/package-registry/blob/dd7b021893aa8d66a5a5fde963d8ff2792a9b8fa/util/package.go#L63 [1]
+  data_streams?: RegistryDataStream[]; // Registry addition [0] [0]: https://github.com/elastic/package-registry/blob/dd7b021893aa8d66a5a5fde963d8ff2792a9b8fa/util/package.go#L65
+}
+interface RegistryOverridePropertyValue {
+  icons?: RegistryImage[];
+  screenshots?: RegistryImage[];
 }
 
+export type RegistryRelease = PackageSpecManifest['release'];
 export interface RegistryImage {
   src: string;
   path: string;
   title?: string;
   size?: string;
   type?: string;
 }
+
 export interface RegistryPolicyTemplate {
   name: string;
   title: string;
   description: string;
-  inputs: RegistryInput[];
+  inputs?: RegistryInput[];
   multiple?: boolean;
 }
-
 export interface RegistryInput {
   type: string;
   title: string;
-  description?: string;
-  vars?: RegistryVarsEntry[];
+  description: string;
   template_path?: string;
+  condition?: string;
+  vars?: RegistryVarsEntry[];
 }
-
 export interface RegistryStream {
   input: string;
   title: string;
@@ -152,15 +153,15 @@ export type RegistrySearchResult = Pick<
   | 'release'
   | 'description'
   | 'type'
-  | 'icons'
-  | 'internal'
   | 'download'
   | 'path'
+  | 'icons'
+  | 'internal'
   | 'data_streams'
   | 'policy_templates'
 >;
 
-export type ScreenshotItem = RegistryImage;
+export type ScreenshotItem = RegistryImage | PackageSpecScreenshot;
 
 // from /categories
 // https://github.com/elastic/package-registry/blob/master/docs/api/categories.json
@@ -172,7 +173,7 @@ export interface CategorySummaryItem {
   count: number;
 }
 
-export type RequirementsByServiceName = Record<ServiceName, ServiceRequirements>;
+export type RequirementsByServiceName = PackageSpecManifest['conditions'];
 export interface AssetParts {
   pkgkey: string;
   dataset?: string;
@@ -241,31 +242,24 @@ export interface RegistryVarsEntry {
 
 // some properties are optional in Registry responses but required in EPM
 // internal until we need them
-interface PackageAdditions {
+export interface EpmPackageAdditions {
   title: string;
   latestVersion: string;
   assets: AssetsGroupedByServiceByType;
   removable?: boolean;
 }
 
+type Merge<FirstType, SecondType> = Omit<FirstType, Extract<keyof FirstType, keyof SecondType>> &
+  SecondType;
+
 // Managers public HTTP response types
 export type PackageList = PackageListItem[];
 
 export type PackageListItem = Installable<RegistrySearchResult>;
 export type PackagesGroupedByStatus = Record<ValueOf<InstallationStatus>, PackageList>;
 export type PackageInfo =
-  | Installable<
-      // remove the properties we'll be altering/replacing from the base type
-      Omit<RegistryPackage, keyof PackageAdditions> &
-        // now add our replacement definitions
-        PackageAdditions
-    >
-  | Installable<
-      // remove the properties we'll be altering/replacing from the base type
-      Omit<ArchivePackage, keyof PackageAdditions> &
-        // now add our replacement definitions
-        PackageAdditions
-    >;
+  | Installable<Merge<RegistryPackage, EpmPackageAdditions>>
+  | Installable<Merge<ArchivePackage, EpmPackageAdditions>>;
 
 export interface Installation extends SavedObjectAttributes {
   installed_kibana: KibanaAssetReference[];

diff --git a/x-pack/plugins/fleet/common/types/models/index.ts b/x-pack/plugins/fleet/common/types/models/index.ts
@@ -10,5 +10,6 @@ export * from './package_policy';
 export * from './data_stream';
 export * from './output';
 export * from './epm';
+export * from './package_spec';
 export * from './enrollment_api_key';
 export * from './settings';
diff --git a/x-pack/plugins/fleet/common/types/models/package_spec.ts b/x-pack/plugins/fleet/common/types/models/package_spec.ts
@@ -0,0 +1,71 @@
+/*
+ * 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 { RegistryPolicyTemplate } from './epm';
+
+// Based on https://github.com/elastic/package-spec/blob/master/versions/1/manifest.spec.yml#L8
+export interface PackageSpecManifest {
+  format_version: string;
+  name: string;
+  title: string;
+  description: string;
+  version: string;
+  license?: 'basic';
+  type?: 'integration';
+  release: 'experimental' | 'beta' | 'ga';
+  categories?: Array<PackageSpecCategory | undefined>;
+  conditions?: PackageSpecConditions;
+  icons?: PackageSpecIcon[];
+  screenshots?: PackageSpecScreenshot[];
+  policy_templates?: RegistryPolicyTemplate[];
+  owner: { github: string };
+}
+
+export type PackageSpecCategory =
+  | 'aws'
+  | 'azure'
+  | 'cloud'
+  | 'config_management'
+  | 'containers'
+  | 'crm'
+  | 'custom'
+  | 'datastore'
+  | 'elastic_stack'
+  | 'google_cloud'
+  | 'kubernetes'
+  | 'languages'
+  | 'message_queue'
+  | 'monitoring'
+  | 'network'
+  | 'notification'
+  | 'os_system'
+  | 'productivity'
+  | 'security'
+  | 'support'
+  | 'ticketing'
+  | 'version_control'
+  | 'web';
+
+export type PackageSpecConditions = Record<
+  'kibana',
+  {
+    version: string;
+  }
+>;
+
+export interface PackageSpecIcon {
+  src: string;
+  title?: string;
+  size?: string;
+  type?: string;
+}
+
+export interface PackageSpecScreenshot {
+  src: string;
+  title: string;
+  size?: string;
+  type?: string;
+}
diff --git a/x-pack/plugins/fleet/common/types/rest_spec/epm.ts b/x-pack/plugins/fleet/common/types/rest_spec/epm.ts
@@ -8,7 +8,7 @@ import {
   AssetReference,
   CategorySummaryList,
   Installable,
-  RegistryPackage,
+  RegistrySearchResult,
   PackageInfo,
 } from '../models/epm';
 
@@ -30,14 +30,7 @@ export interface GetPackagesRequest {
 }
 
 export interface GetPackagesResponse {
-  response: Array<
-    Installable<
-      Pick<
-        RegistryPackage,
-        'name' | 'title' | 'version' | 'description' | 'type' | 'icons' | 'download' | 'path'
-      >
-    >
-  >;
+  response: Array<Installable<RegistrySearchResult>>;
 }
 
 export interface GetLimitedPackagesResponse {

diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/epm/components/requirements.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/epm/components/requirements.tsx
@@ -7,7 +7,7 @@
 import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiText, EuiTextColor, EuiTitle } from '@elastic/eui';
 import React, { Fragment } from 'react';
 import styled from 'styled-components';
-import { RequirementsByServiceName, entries } from '../../../types';
+import { RequirementsByServiceName, ServiceName, entries } from '../../../types';
 import { ServiceTitleMap } from '../constants';
 import { Version } from './version';
 
@@ -23,6 +23,14 @@ const StyledVersion = styled(Version)`
   font-size: ${(props) => props.theme.eui.euiFontSizeXS};
 `;
 
+// both our custom `entries` and this type seem unnecessary & duplicate effort but they work for now
+type RequirementEntry = [
+  Extract<ServiceName, 'kibana'>,
+  {
+    version: string;
+  }
+];
+
 export function Requirements(props: RequirementsProps) {
   const { requirements } = props;
 
@@ -40,15 +48,15 @@ export function Requirements(props: RequirementsProps) {
           </EuiTitle>
         </EuiFlexItem>
       </FlexGroup>
-      {entries(requirements).map(([service, requirement]) => (
+      {entries(requirements).map(([service, requirement]: RequirementEntry) => (
         <EuiFlexGroup key={service}>
           <EuiFlexItem grow={true}>
             <EuiTextColor color="subdued" key={service}>
               {ServiceTitleMap[service]}:
             </EuiTextColor>
           </EuiFlexItem>
           <EuiFlexItem grow={false}>
-            <StyledVersion version={requirement.versions} />
+            <StyledVersion version={requirement.version} />
           </EuiFlexItem>
         </EuiFlexGroup>
       ))}

diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/epm/constants.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/epm/constants.tsx
@@ -29,8 +29,7 @@ export const AssetTitleMap: Record<AssetType, string> = {
   map: 'Map',
 };
 
-export const ServiceTitleMap: Record<ServiceName, string> = {
-  elasticsearch: 'Elasticsearch',
+export const ServiceTitleMap: Record<Extract<ServiceName, 'kibana'>, string> = {
   kibana: 'Kibana',
 };
 

diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/epm/hooks/use_links.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/epm/hooks/use_links.tsx
@@ -6,7 +6,7 @@
 import { useStartServices } from '../../../hooks/use_core';
 import { PLUGIN_ID } from '../../../constants';
 import { epmRouteService } from '../../../services';
-import { RegistryImage } from '../../../../../../common';
+import { PackageSpecIcon, PackageSpecScreenshot, RegistryImage } from '../../../../../../common';
 
 const removeRelativePath = (relativePath: string): string =>
   new URL(relativePath, 'http://example.com').pathname;
@@ -15,12 +15,19 @@ export function useLinks() {
   const { http } = useStartServices();
   return {
     toAssets: (path: string) => http.basePath.prepend(`/plugins/${PLUGIN_ID}/assets/${path}`),
-    toPackageImage: (img: RegistryImage, pkgName: string, pkgVersion: string): string =>
-      img.src
-        ? http.basePath.prepend(
-            epmRouteService.getFilePath(`/package/${pkgName}/${pkgVersion}${img.src}`)
-          )
-        : http.basePath.prepend(epmRouteService.getFilePath(img.path)),
+    toPackageImage: (
+      img: PackageSpecIcon | PackageSpecScreenshot | RegistryImage,
+      pkgName: string,
+      pkgVersion: string
+    ): string | undefined => {
+      const sourcePath = img.src
+        ? `/package/${pkgName}/${pkgVersion}${img.src}`
+        : 'path' in img && img.path;
+      if (sourcePath) {
+        const filePath = epmRouteService.getFilePath(sourcePath);
+        return http.basePath.prepend(filePath);
+      }
+    },
     toRelativeImage: ({
       path,
       packageName,