Add owner.type field to indicate package support level by taylor-swanson · Pull Request #568 · elastic/package-spec

taylor-swanson · 2023-07-20T14:53:09Z

What does this PR do?

Add new owner.type field to indicate the support level of a package in the manifest.yml file.
The 'elastic' value indicates the package is built and supported by Elastic
The 'partner' value indicates the package is built and supported by a partner/vendor and may include involvement from Elastic
The 'community' value indicates the package is built and supported by non-Elastic community members
The field is currently not required

Why is it important?

Identifies the expected support owner and level of our packages. Other areas, such as the Kibana Integrations UI, can display notices on the support level or provide for filtering to only show Elastic-supported packages, for instance.

Checklist

I have added test packages to test/packages that prove my change is effective.
I have added an entry in spec/changelog.yml.

Related issues

Relates [Epic] Tagging Community/Partner Integrations integrations#6569

- Add new owner.type field to indicate the support level of a package. - The 'elastic' value indicates the package is built and supported by Elastic - The 'vendor' value indiciates the package is built and supported by a vendor and may include involvement from Elastic - The 'community' value indicates the package is built and supported by non-Elastic community members - The field is currently not required

jsoriano · 2023-07-26T08:17:27Z

spec/integration/manifest.spec.yml

          type: string
          pattern: '^(([a-zA-Z0-9-]+)|([a-zA-Z0-9-]+\/[a-zA-Z0-9-]+))$'
+        type:
+          description: Describes who owns the package and the level of support that is provided.


We should probably define what to expect when this value is not set.

In principle I see two options:

elastic as default, as most existing packages are ours. This may give problems if publishers forget to set the value in the future.

Add this field starting on next package-spec version (2.10.0). Assume community as default from this version of the spec, and elastic for current packages. We could add something in our CI to ensure that we set the field.

++ to defining a default.

I prefer the second option of having elastic needing set explicitly for future packages we release.

I definitely agree on setting a default and also agree with defaulting to community for the next package-spec version. For existing packages, I think I saw a before constraint that we can set to satisfy this:

elastic for current packages.

I saw this, for example:

- before: 2.3.0 patch: - op: add path: "/properties/release" value: description: The stability of the package (deprecated, use prerelease tags in the version). deprecated: true # See https://github.com/elastic/package-spec/issues/225 type: string enum: - experimental - beta - ga default: ga examples: - experimental

Would I do something similar for owner.type and default it to elastic when the version is before 2.10.0?

Basically, this:

# JSON patches for newer versions should be placed on top versions: - before: 2.10.0 patch: - op: remove path: "/definitions/conditions/properties/elastic/properties/capabilities" + - op: add + path: "/definitions/owner/properties/type" + value: + description: Describes who owns the package and the level of support that is provided. + type: string + default: elastic + enum: + - elastic + - partner + - community + examples: + - community - before: 2.3.0

Well, you could replace only the default, with something like:

+ - op: replace + path: "/definitions/owner/properties/type/default" + value: elastic

Or directly remove the definition so it cannot be used in older versions to avoid confusion:

+ - op: remove + path: "/definitions/owner/properties/type"

I'm leaning towards the "replacing the default" option so that way a value is always present, rather than the definition being absent in some cases. If you don't think that'd be a concern, then I'm okay with removing the definition as well. Either change seems pretty straightforward to add.

taylor-swanson · 2023-08-10T12:39:37Z

@jsoriano, looks like 2.10.0 was just released. I'd imagine we'd want this going into 2.11.0 now and not 2.10.1?

jsoriano · 2023-08-10T13:47:47Z

@jsoriano, looks like 2.10.0 was just released. I'd imagine we'd want this going into 2.11.0 now and not 2.10.1?

Hi, I am very sorry that we missed this change. Yes, In principle I am afraid that it will have to wait for 2.11.0.

jsoriano

Sorry we didn't review it before 2.10.0 release!

jsoriano · 2023-08-10T13:50:11Z

spec/integration/manifest.spec.yml

          type: string
          pattern: '^(([a-zA-Z0-9-]+)|([a-zA-Z0-9-]+\/[a-zA-Z0-9-]+))$'
+        type:
+          description: Describes who owns the package and the level of support that is provided.


Nit. Please add to the description what each possible value is intended for.

jsoriano · 2023-08-10T13:50:49Z

spec/integration/manifest.spec.yml

    patch:
      - op: remove
        path: "/definitions/conditions/properties/elastic/properties/capabilities"
+      - op: replace


This will need to be moved to a new section - before: 2.11.0.

jsoriano · 2023-08-10T13:53:12Z

test/packages/bad_owner_type/manifest.yml

+        title: Collect sample logs from instances
+        description: Collecting sample logs
+owner:
+  type: invalid


Please add also an owner to the good_v2 test package.

jsoriano · 2023-08-10T14:10:40Z

spec/changelog.yml

+  changes:
+  - description: Add owner.type field to indicate package support level
+    type: enhancement
+    link: https://github.com/elastic/package-spec/pull/568


Please, remove this block as it won't be needed.

Not quite sure I understand, do you want me to put the changelog entry under 2.10.1-next instead?

No, we have to remove at some point the 2.10.1-next block, but no worries, we can also remove it later.

I'll put it under 2.10.1-next so at least the entry is in there, but we can remove/rearrange later like you say.

jsoriano · 2023-08-10T14:10:58Z

spec/integration/manifest.spec.yml

+            and maintained by Elastic. The 'partner' value indicates that the
+            package is built and maintained by a partner vendor and may include
+            involvement from Elastic. The 'community' value indicates the package
+            is built and maintained by non-Elastic community members.


👍 thanks

elasticmachine · 2023-08-10T17:03:40Z

💚 Build Succeeded

Buildkite Build
Commit: 288d688

History

💚 Build #129 succeeded bcc25d6
💚 Build #83 succeeded 79a0567
💚 Build #82 succeeded 9a10017
💚 Build #69 succeeded 5e1bfd0
💚 Build #68 succeeded 2d634ae

cc @taylor-swanson

jsoriano · 2023-08-15T08:40:01Z

Let's merge this before we forget again 🙂

taylor-swanson self-assigned this Jul 20, 2023

Update changelog

2d634ae

taylor-swanson mentioned this pull request Jul 20, 2023

[Epic] Tagging Community/Partner Integrations elastic/integrations#6569

Closed

3 tasks

Add test

5e1bfd0

jsoriano reviewed Jul 26, 2023

View reviewed changes

taylor-swanson added 3 commits July 26, 2023 14:57

Set a default value

b2f7a1b

Merge branch 'main' into owner-type-tagging

9a10017

Older versions use elastic by default

79a0567

taylor-swanson marked this pull request as ready for review July 27, 2023 13:01

taylor-swanson requested a review from a team as a code owner July 27, 2023 13:01

jsoriano reviewed Aug 10, 2023

View reviewed changes

taylor-swanson added 2 commits August 10, 2023 09:06

Review comments, realign to 2.11.0

3c40f56

Merge branch 'main' into owner-type-tagging

bcc25d6

jsoriano previously approved these changes Aug 10, 2023

View reviewed changes

Changelog

288d688

taylor-swanson dismissed jsoriano’s stale review via 288d688 August 10, 2023 16:54

jsoriano approved these changes Aug 10, 2023

View reviewed changes

jsoriano merged commit 32eb741 into elastic:main Aug 15, 2023

taylor-swanson deleted the owner-type-tagging branch August 15, 2023 12:48

taylor-swanson mentioned this pull request Sep 18, 2023

Set owner.type field as required #604

Merged

2 tasks

jsvd mentioned this pull request Jan 7, 2025

[Change Proposal] Add new property to integration to show if their processing can run on Logstash #824

Open

Conversation

taylor-swanson commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Related issues

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!

taylor-swanson commented Aug 10, 2023

Uh oh!

jsoriano commented Aug 10, 2023

Uh oh!

jsoriano left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 10, 2023

💚 Build Succeeded

History

Uh oh!

jsoriano commented Aug 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

taylor-swanson commented Jul 20, 2023 •

edited

Loading