[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec #84946

jfsiii · 2020-12-03T20:29:58Z

Summary

Create a TS type for package-spec 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.

kibana/x-pack/plugins/fleet/common/types/models/epm.ts

Lines 71 to 80 in 5f2c4a5

    
           export type InstallablePackage = RegistryPackage | ArchivePackage; 
        
           export type ArchivePackage = PackageSpecManifest & 
        
             // should an uploaded package be able to specify `internal`? 
        
             Pick<RegistryPackage, 'readme' | 'assets' | 'data_streams' | 'internal'>; 
        
           export type RegistryPackage = PackageSpecManifest & 
        
             Partial<RegistryOverridesToOptional> & 
        
             RegistryAdditionalProperties & 
        
             RegistryOverridePropertyValue;

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. Removed the required format_version and re-zipped.

screenshots show errors if catches in a mock PackageInfo object

using `requirement`; not `conditions`

using `versions`; not `version`

including `elasticsearch` when spec says it' not valid yet

screenshots showing editor autocomplete for registry response values like categories

screenshot showing type check preventing adding properties which aren't explicitly listed as "additions"

Checklist

Unit or functional tests were updated or added to match the most common scenarios

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. 3 TS errors and I think they're all expected to be fixed by elastic#84944 ERROR x-pack failed x-pack/plugins/fleet/public/applications/fleet/hooks/use_package_icon_type.ts:45:66 - error TS2339: Property 'path' does not exist on type 'PackageSpecIcon'. 45 const localIconSrc = Array.isArray(svgIcons) && (svgIcons[0].path || svgIcons[0].src); ~~~~ x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/overview_panel.tsx:13:24 - error TS2339: Property 'readme' does not exist on type 'PackageInfo'. 13 const { screenshots, readme, name, version } = props; ~~~~~~ x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/overview_panel.tsx:18:36 - error TS2322: Type 'PackageSpecScreenshot[] | (PackageSpecScreenshot[] & RegistryImage[])' is not assignable to type 'RegistryImage[]'. Type 'PackageSpecScreenshot[]' is not assignable to type 'RegistryImage[]'. Property 'path' is missing in type 'PackageSpecScreenshot' but required in type 'RegistryImage'. 18 {screenshots && <Screenshots images={screenshots} />} ~~~~~~ x-pack/plugins/fleet/common/types/models/epm.ts:115:3 115 path: string; ~~~~ 'path' is declared here. x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/screenshots.tsx:14:3 14 images: ScreenshotItem[]; ~~~~~~ The expected type comes from property 'images' which is declared here on type 'IntrinsicAttributes & ScreenshotProps' Found 3 errors.

x-pack/plugins/fleet/common/types/models/package_spec.ts

neptunian · 2020-12-04T15:38:29Z

Thanks for cleaning up and organizing the types! 🎉 Looks good once CI passes.

jfsiii · 2020-12-06T21:41:06Z

@elasticmachine merge upstream

kibanamachine · 2020-12-06T21:41:08Z

merge conflict between base and head

jfsiii · 2020-12-06T23:55:39Z

x-pack/plugins/fleet/common/types/models/epm.ts

  title: string;
  latestVersion: string;
  assets: AssetsGroupedByServiceByType;
  removable?: boolean;
 }

+type Merge<FirstType, SecondType> = Omit<FirstType, Extract<keyof FirstType, keyof SecondType>> &


From https://github.com/sindresorhus/type-fest/#utilities. Can move elsewhere later if needed.

jfsiii · 2020-12-06T23:57:07Z

x-pack/plugins/fleet/common/types/rest_spec/epm.ts

-      >
-    >
-  >;
+  response: Array<Installable<RegistrySearchResult>>;


Reference existing type instead of duplicating the definition

jfsiii · 2020-12-06T23:58:38Z

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> = {


Package spec (currently) limits values to kibana

jfsiii · 2020-12-07T00:04:06Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/hooks/use_links.tsx

+    ): string | undefined => {
+      const sourcePath = img.src
+        ? `/package/${pkgName}/${pkgVersion}${img.src}`
+        : 'path' in img && img.path;


path doesn't exist in PackageSpec*

jfsiii · 2020-12-07T00:05:04Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/screenshots.tsx

@@ -69,18 +70,20 @@ export function Screenshots(props: ScreenshotProps) {
            <EuiSpacer />
          </NestedEuiFlexItem>
        )}
-        <NestedEuiFlexItem>
-          {/* By default EuiImage sets width to 100% and Figure to 22.5rem for size=l images,
+        {screenshotUrl && (


make sure we have an image URL before attempting to render it.

elasticmachine · 2020-12-07T00:18:18Z

Pinging @elastic/ingest-management (Team:Ingest Management)

jfsiii · 2020-12-07T00:18:59Z

x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts

@@ -29,7 +28,11 @@ describe('Fleet - packageToPackagePolicy', () => {
        map: [],
      },
    },
-    status: installationStatuses.NotInstalled,


Not strictly required for this change. Only because I was in here and it's still type safe but w/one less import

jfsiii · 2020-12-07T01:55:47Z

@elasticmachine merge upstream

jfsiii · 2020-12-07T12:05:11Z

@elasticmachine merge upstream

jfsiii · 2020-12-07T13:57:15Z

x-pack/plugins/security_solution/common/endpoint/generate_data.ts

@@ -1310,6 +1310,7 @@ export class EndpointDocGenerator {
        },
      ],
      status: 'installed',
+      release: 'ga',


release key is required by package-spec and can be one of experimental, beta, or ga

jfsiii · 2020-12-07T13:58:16Z

x-pack/plugins/security_solution/common/endpoint/generate_data.ts

@@ -1298,7 +1298,7 @@ export class EndpointDocGenerator {
      title: 'Elastic Endpoint',
      version: '0.5.0',
      description: 'This is the Elastic Endpoint package.',
-      type: 'solution',
+      type: 'integration',


integration is the only valid type value

paul-tavares

LGTM @jfsiii
Thanks for the changes to the Endpoint generator

…ith spec (elastic#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

…ith spec (#84946) (#85139) ## 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

kibanamachine · 2020-12-07T22:07:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 3f39ec4
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

X-Pack Jest Unit Tests

Test Failures

X-Pack Jest Tests.x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/datatypes.Mappings editor: scaled float datatype should require a scaling factor to be provided

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 1 times on tracked branches: https://github.com/elastic/kibana/issues/85135

Stack Trace

Error: thrown: "Exceeded timeout of 5000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."
    at /dev/shm/workspace/parallel/9/kibana/x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/datatypes/scaled_float_datatype.test.tsx:44:3
    at _dispatchDescribe (/dev/shm/workspace/kibana/node_modules/jest-circus/build/index.js:67:26)
    at describe (/dev/shm/workspace/kibana/node_modules/jest-circus/build/index.js:30:5)
    at Object.<anonymous> (/dev/shm/workspace/parallel/9/kibana/x-pack/plugins/index_management/public/application/components/mappings_editor/__jest__/client_integration/datatypes/scaled_float_datatype.test.tsx:22:1)
    at Runtime._execModule (/dev/shm/workspace/kibana/node_modules/jest-runtime/build/index.js:1299:24)
    at Runtime._loadModule (/dev/shm/workspace/kibana/node_modules/jest-runtime/build/index.js:898:12)
    at Runtime.requireModule (/dev/shm/workspace/kibana/node_modules/jest-runtime/build/index.js:746:10)
    at jestAdapter (/dev/shm/workspace/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:106:13)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at runTestInternal (/dev/shm/workspace/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/dev/shm/workspace/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/dev/shm/workspace/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fleet`	449	450	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	1.1MB	1.1MB	+61.0B

Distributable file count

id	before	after	diff
`default`	46920	47681	+761

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`fleet`	374.9KB	375.5KB	+585.0B

History

💚 Build #92315 succeeded 3f39ec4
💔 Build #92304 failed 7fa6a05
💔 Build #92244 failed aa6502f
💔 Build #92242 failed 5f2c4a5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

jfsiii requested review from skh and neptunian December 3, 2020 20:29

Add comment with link to package spec

b36871e

jfsiii commented Dec 4, 2020

View reviewed changes

x-pack/plugins/fleet/common/types/models/package_spec.ts Show resolved Hide resolved

neptunian approved these changes Dec 4, 2020

View reviewed changes

John Schulz added 4 commits December 6, 2020 16:44

Merge branch 'master' into separate-spec-from-registry-types

53fc1ca

InstallablePackage = RegistryPackage | ArchivePackage

6c6f863

Update test to reflect new required properties

216f898

Remove optional properties added earlier

09cce23

jfsiii commented Dec 6, 2020

View reviewed changes

jfsiii commented Dec 7, 2020

View reviewed changes

jfsiii marked this pull request as ready for review December 7, 2020 00:13

jfsiii requested a review from a team as a code owner December 7, 2020 00:13

jfsiii requested a review from a team December 7, 2020 00:13

jfsiii requested a review from a team as a code owner December 7, 2020 00:13

Use string vs object for status value

efc73f9

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Dec 7, 2020

jfsiii commented Dec 7, 2020

View reviewed changes

Export a PackageSpec type vs repeating it

5f2c4a5

jfsiii self-assigned this Dec 7, 2020

jfsiii added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 7, 2020

jfsiii added the v8.0.0 label Dec 7, 2020

kibanamachine and others added 2 commits December 6, 2020 20:55

Merge branch 'master' into separate-spec-from-registry-types

aa6502f

Update missing_field zip

7fa6a05

Merge branch 'master' into separate-spec-from-registry-types

3f39ec4

skh approved these changes Dec 7, 2020

View reviewed changes

jfsiii commented Dec 7, 2020

View reviewed changes

paul-tavares approved these changes Dec 7, 2020

View reviewed changes

jfsiii merged commit ef7367d into elastic:master Dec 7, 2020

jfsiii mentioned this pull request Dec 7, 2020

[7.x] [Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec (#84946) #85139

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec #84946

[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec #84946

jfsiii commented Dec 3, 2020 •

edited

Loading

neptunian commented Dec 4, 2020 •

edited

Loading

jfsiii commented Dec 6, 2020

kibanamachine commented Dec 6, 2020

jfsiii Dec 6, 2020

jfsiii Dec 6, 2020

jfsiii Dec 6, 2020

jfsiii Dec 7, 2020

jfsiii Dec 7, 2020

elasticmachine commented Dec 7, 2020

jfsiii Dec 7, 2020

jfsiii commented Dec 7, 2020

jfsiii commented Dec 7, 2020

jfsiii Dec 7, 2020

jfsiii Dec 7, 2020

paul-tavares left a comment

kibanamachine commented Dec 7, 2020 •

edited

Loading

Standard Out

Stack Trace

	export type InstallablePackage = RegistryPackage \| ArchivePackage;

	export type ArchivePackage = PackageSpecManifest &
	// should an uploaded package be able to specify `internal`?
	Pick<RegistryPackage, 'readme' \| 'assets' \| 'data_streams' \| 'internal'>;

	export type RegistryPackage = PackageSpecManifest &
	Partial<RegistryOverridesToOptional> &
	RegistryAdditionalProperties &
	RegistryOverridePropertyValue;

[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec #84946

[Fleet][EPM] Add TS type for package-spec. Clarify EPR relationship with spec #84946

Conversation

jfsiii commented Dec 3, 2020 • edited Loading

Summary

Checklist

neptunian commented Dec 4, 2020 • edited Loading

jfsiii commented Dec 6, 2020

kibanamachine commented Dec 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Dec 7, 2020

Choose a reason for hiding this comment

jfsiii commented Dec 7, 2020

jfsiii commented Dec 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

kibanamachine commented Dec 7, 2020 • edited Loading

💔 Build Failed

Failed CI Steps

Standard Out

Stack Trace

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

jfsiii commented Dec 3, 2020 •

edited

Loading

neptunian commented Dec 4, 2020 •

edited

Loading

kibanamachine commented Dec 7, 2020 •

edited

Loading