Add field for source license and condition for subscription #355
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
| @@ -4,8 +4,11 @@ title: Good package | |||
| description: This package is good. | |||
| version: 1.0.0 | |||
| type: integration | |||
| source: | |||
There was a problem hiding this comment.
Do we need specifically a source property? Do you expect other properties included in the source?
There was a problem hiding this comment.
Not needed, but also we cannot use license as is the current problematic name.
We could have other metadata about the source, for example the url.
| @@ -4,8 +4,11 @@ title: Good package | |||
| description: This package is good. | |||
| version: 1.0.0 | |||
| type: integration | |||
| source: | |||
| license: "Apache-2.0" | |||
There was a problem hiding this comment.
Single license property violates this comment. How about creating licensing structure and keeping an array of licenses with path refs there?
There was a problem hiding this comment.
Umm, I consider this property implements this comment. This is a value, from an enum of allowed values. How does it violate the comment? (The enum would be the one here https://github.com/elastic/package-spec/pull/355/files#diff-fb1ef845cdf5e9bd952636bf029c091248cedcccee3645a86c909c2b21ea3acaR137)
There was a problem hiding this comment.
How would the licensing structure be? Something like this?
licensing:
license: 'Apache-2.0'
subscription: 'basic'
There was a problem hiding this comment.
In case we have multiple licenses per package we could use that structure:
licensing:
licenses: [
name: Apache-2.0,
path: LICENSE.txt
]
subscription: basic
Sometimes there is also NOTICE.txt file, which seems to be related to package license.
There was a problem hiding this comment.
I would avoid having multiple licenses in the same package. What would be the use case of this? To have packages with Apache licensed code and Elastic licensed one? Is this something we want to support?
If we support this we also need some way to indicate what parts of the package are licensed with each license.
Regarding the links to text files, this comment talks against this, and I think I agree. We can have a limited list of supported licenses and elastic-package (or fleet in the UI) can be used to provide the full texts where needed.
| conditions: | ||
| kibana.version: '^7.9.0' | ||
| elastic.subscription: 'basic' |
There was a problem hiding this comment.
Hm... it's a tough one. It is a condition or requirement to have a basic subscription. On the other hand, conditions will become a bag with various properties, which isn't great too. Maybe it can be moved to the licensing structure I proposed above?
There was a problem hiding this comment.
We need different fields for license and subscription.
The subscription is a requirement, this is why I thought that putting it as condition could make sense. I am ok with moving it to its own field though.
There was a problem hiding this comment.
Both options sound good, conditions may also contain regular expressions, for example, basic || special. Maybe we can ask other folks about their preferences. @joshdover
There was a problem hiding this comment.
I'd prefer we keep it simple for now and not allow packages to define any special licensing structure. Kibana's existing licensing logic assumes a very basic, hierarchical structure where each tier includes all the features of the tier below it, expressed as a license.hasAtLeast('platinum') API. I don't think we should allow packages to do anything more sophisticated at this point, but leave the spec open to modification later.
I think if we start with an enum today, we should be able to expand it to more sophisticated things like regex in the future.
Shouldn't we go ahead and add the other enum values: basic, gold, platinum, enterprise?
There was a problem hiding this comment.
I was also thinking on the "at least" semantics.
Regarding basic || special, do we have any feature now that is included on different subscriptions? This would be for example to have something in gold and enterprise that is not included in platinum. This sounds confusing.
There was a problem hiding this comment.
Shouldn't we go ahead and add the other enum values:
basic,gold,platinum,enterprise?
👍 added.
What does this PR do?
source.licensefield, with the actual license of the package.elastic.subscriptioncondition, that allows to optionally declare a required subscription to install a package.licensefield.Why is it important?
Current "license" field is referring to the subscription level, and there is no way to indicate the actual license of the package.
See #298 for more information.
Checklist
test/packagesthat prove my change is effective.versions/N/changelog.yml.Related issues