Adding initial spec files by ycombinator · Pull Request #1 · elastic/package-spec

ycombinator · 2020-07-28T23:59:43Z

This PR adds specification files for the initial package specification (format_version = 1.0.0, which is what all packages today are using). The specification files describe:

the expected folder structure of packages, including expected files within these folders; and
the expected structure of the expected files' contents.

For 1. above, a schema very similar to JSON Schema is used (but written as YAML for readability), with two notable differences:

The type field can be either folder or file, and
A new field, contents is introduced to (recursively) describe the contents of folders (i.e. when type == folder).

For 2. above, JSON Schema is used (but, again, written as YAML for readability).

Additionally, this PR adds a README.md file explaining the specification format.

Resolves #2.

Author's TODO

Flesh out package folders+expected files specification (1. above)
Flesh out specification for expected files' contents (2. above)
- Main package manifest file
- Dataset manifest file
- Various dataset fields files
- CHANGELOG file
Document specification format in README

versions/1.0.0/dataset/fields/fields.spec.yml

ycombinator · 2020-07-30T18:31:54Z

versions/1.0.0/dataset/fields/fields.spec.yml

+      type: string
+      enum:
+      - custom
+    default_field:


@jonathan-buttner I saw this default_field field used here https://github.com/elastic/package-registry/blob/98d7184cfe5b1d271e1bef848a5720073959888e/testdata/package/ecs_style_dataset/0.0.1/dataset/foo/fields/fields.yml#L16. Could you tell me what it is meant for so I could appropriately describe it in this schema here? Thanks!

~~@ycombinator I think we got rid of default_field and it was replaced by show_user and some others. So this is only here because it was not cleaned up. Found it also in other places.~~

Sorry, ignore my comment above, just realised this is fields.yml.

We can ignore this one too I think:

It is outputted by the ECS beats tooling:
https://github.com/elastic/ecs/blob/master/scripts/generators/beats_default_fields_whitelist.yml

https://github.com/elastic/ecs/blob/master/generated/beats/fields.ecs.yml#L77

I'm not even really sure what it is used for 🤷

@ruflin @mtojek Do you have any idea what the purpose of this default_field is?

No idea at all, I think you can safely ignore/remove this field from spec.

Okay, I will remove it from the spec, similar to the level and group fields which were removed in dd76b6d.

Removed in 396721e.

versions/1.0.0/manifest.spec.yml

masci

Versioning looks very good to me, thanks for working on it!

README.md

ruflin · 2020-08-03T11:08:47Z

@ycombinator The registry running under epr-epxerimental.elastic.co is for 7.8. It already contains the spec format 1.0.0 (which of course is wrong). Packages under epr.elastic.co which is for >=7.9 also contains 1.0.0 even though the two package formats are not compatible. In this case, it does not matter as the format is not read out and does not break anything. What I'm getting at with my above comment, is that there is a chance the the format we ship in 7.9 is not the final one and we do a breaking change again with the same trick we did for 7.8 with a separate registry. What this means for the spec is that even though you defined 1.0.0 above and it could imply all follow up changes are not breaking, it is likely not the case as what we have today in packages is not necessarly 1.0.0.

All of this should not hold back this PR from merging.

Co-authored-by: Massimiliano Pippi <mpippi@gmail.com>

ycombinator · 2020-08-03T18:45:34Z

Thanks for the explanation, @ruflin.

I agree that the breaking changes should not hold back this PR. However, it means that we can't use the 1.0.0 spec for validating packages just yet, until all breaking changes have "settled". I have already labeled this spec version as 1.0.0-alpha1 in the versions/1/changelog.yml file and in the version field of the versions/1/spec.yml file. Additionally I will add a warning to the top of the README stating that the spec is under active development and not ready for use just yet. Once the breaking changes have settled we can remove the alpha1 label, remove the warning from the README, and start using the spec for validating packages. Needless to say, it would be good to get to this state sooner rather than later.

I looked through the comments in this PR and there are a few that are awaiting your responses. I've gathered them below for your convenience. If you could take a few minutes to respond to them, it would help move along this PR. Thanks!

jonathan-buttner · 2020-08-04T12:50:10Z

versions/1/dataset/fields/spec.yml

+  description: Base fields definitions
+  type: file
+  name: "base-fields.yml"
+  required: true


Hmm can we make this required: false the endpoint package doesn't include this file. We include the fields directly in fields.yml. Should we allow the package developer to call the file whatever they want? Would it be easy to just validate that the directory isn't empty and that the file conforms to the fields.spec.yml?

Yeah, I'm happy to change this to whatever matches the actual usage/parsing of these files. I don't know what that is, though! Are the files merged together somehow? If so, is there some ordering/precedence taken into account while merging?

The file can have any name and merging order should not matter, at least so far.

Fixed in 076597f.

versions/1/dataset/manifest.spec.yml

ruflin · 2020-08-05T06:59:15Z

@jonathan-buttner One I couldn't answer in line (not sure why) was around integrations vs solution. We introduced solutions as in the beginning we expected to never show the endpoint package to users but this changed. So having it as integrations now makes sense and we can get rid of a an option.

@ycombinator I have not been very good in following up on all comments, but please do not block on me to get this in. There will be breaking changes in this spec for the upcoming dataset -> data_stream change. The part I'm most looking forward to is start validating packages against it and iterate on it. As soon as we get there, we should challenge again every single entry in the spec.

Ship it.

ycombinator · 2020-08-05T10:18:18Z

Many thanks to @jonathan-buttner, @mtojek, @masci, and @ruflin for reviewing this PR so thoroughly! I believe all your concerns have been addressed at this point.

The initial spec in this PR may not be 100% accurate yet but I think it's in decent enough shape to get us started. I fully expect us to iterate on this in the coming weeks, especially once we start validating packages against it!

To indicate that this initial spec is not quite ready for general consumption, I have done two things:

versioned the spec as 1.0.0-alpha1, and
added a warning at the top of the README

With that, I'm planning to merge this PR and then make this repository public later today. If anyone has any last-minute concerns please bring them up within the next few hours. Thank you!

ruflin

Ship it!

versions/1/dataset/manifest.spec.yml

mtojek · 2020-08-05T11:23:57Z

versions/1/dataset/manifest.spec.yml

        - title
        - description
        - input
+    # https://github.com/elastic/package-registry/blob/v0.8.0/util/dataset.go#L48


I believe it would be safer to refer through a commit id.

I'm actually considering removing these comments. I had put them in there for my own benefit to make sure I had accounted for most of the struct fields in the spec.

Removed in ac61112.

versions/1/dataset/manifest.spec.yml

mtojek

Ship it! I wonder if there is a way to enforce required fields in the spec. It might be part of the validation library.

Side note: if they are required to be present in all definitions, why don't we keep them aside from fields.yml

ycombinator added 15 commits July 28, 2020 16:58

Adding initial spec files

394b1cb

Renaming version folder name

2967350

Updating README for version folder name change

bdffdfa

Adding header comments to spec files

5558e48

Clarifying syntax vs. semantics in README

2419161

Fleshing out package manifest.yml spec

eb9a53e

Updating README

904bc9b

Making more similar to JSON schema

d0d5255

Moving version folder under versions/ + updating README to match

c2a1e1a

Fleshing out some more specs

5c81724

Add pattern for package name

7eff101

Updating README

10449e0

Resolving some TODOs

fe691af

Fleshing out fields spec

73b7e4d

Fleshing out CHANGELOG spec

367be0d

ycombinator marked this pull request as ready for review July 30, 2020 18:24

ycombinator requested review from mtojek and ruflin July 30, 2020 18:24

ycombinator commented Jul 30, 2020

View reviewed changes

versions/1.0.0/dataset/fields/fields.spec.yml Outdated Show resolved Hide resolved

ycombinator commented Jul 30, 2020

View reviewed changes

versions/1.0.0/dataset/fields/fields.spec.yml Outdated Show resolved Hide resolved

Updating description for multi field

e3cc793

ycombinator commented Jul 30, 2020

View reviewed changes

ycombinator added 4 commits July 30, 2020 11:41

Adding yaml as supported variable type

18863b1

Document show_user

f0ab0e9

Add TODO for addressing spec duplication later

08e40c7

Fix inconsistency in description

b0c05d6

ycombinator commented Jul 30, 2020

View reviewed changes

versions/1.0.0/manifest.spec.yml Outdated Show resolved Hide resolved

ycombinator added 3 commits July 30, 2020 11:52

Updating README

431d3c7

Introducing contents + renaming specRef to $ref

5f4f5c1

Making file contents schemas strict

fd5303c

ycombinator added 2 commits August 1, 2020 05:22

Updating version fields' regexes

6c38cc1

Use only basic license for now

e585265

masci approved these changes Aug 3, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

Fixing typos

930d297

Co-authored-by: Massimiliano Pippi <mpippi@gmail.com>

ycombinator added 4 commits August 3, 2020 11:48

Adding WARNING to README

e4b6e97

Adding more ES field datatypes

d475a6d

Cross referencing spec against package-registry struct fields

6024684

Added index_pattern folder to kibana folder spec

e315c0d

jonathan-buttner reviewed Aug 4, 2020

View reviewed changes

Removing default_field from spec

396721e

ycombinator added 4 commits August 5, 2020 02:57

Updating fields folder spec

076597f

Removing solution fields/values

4085ee2

Adding elasticsearch section to dataset manifest spec

3bb6f7c

Linting spec YAMLs

d99805f

ruflin approved these changes Aug 5, 2020

View reviewed changes

mtojek reviewed Aug 5, 2020

View reviewed changes

versions/1/dataset/manifest.spec.yml Outdated Show resolved Hide resolved

mtojek reviewed Aug 5, 2020

View reviewed changes

versions/1/dataset/manifest.spec.yml Outdated Show resolved Hide resolved

mtojek self-requested a review August 5, 2020 11:25

mtojek approved these changes Aug 5, 2020

View reviewed changes

ycombinator added 2 commits August 5, 2020 08:18

Fixing typos

bed02b2

Removing development-time comments

ac61112

ycombinator merged commit 78fa992 into elastic:master Aug 5, 2020

ycombinator deleted the init-spec branch August 5, 2020 23:50

rw-access pushed a commit to rw-access/package-spec that referenced this pull request Mar 23, 2021

Add License file (elastic#1)

809a6af

djptek mentioned this pull request Mar 26, 2021

Adjust terminology - change whitelist to allowlist elastic/ecs#1315

Merged

Conversation

ycombinator commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Author's TODO

Uh oh!

Uh oh!

Uh oh!

ycombinator Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruflin Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

masci left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruflin commented Aug 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ycombinator commented Aug 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ycombinator Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ruflin commented Aug 5, 2020

Uh oh!

ycombinator commented Aug 5, 2020

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mtojek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

ycombinator commented Jul 28, 2020 •

edited

Loading

ycombinator Jul 30, 2020 •

edited

Loading

ruflin Jul 31, 2020 •

edited

Loading

ruflin commented Aug 3, 2020 •

edited

Loading

ycombinator commented Aug 3, 2020 •

edited

Loading

ycombinator Aug 5, 2020 •

edited

Loading

mtojek left a comment •

edited

Loading