Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add License and Subscription in integration details section #298

Closed
4 tasks done
akshay-saraswat opened this issue Mar 17, 2022 · 14 comments
Closed
4 tasks done

Add License and Subscription in integration details section #298

akshay-saraswat opened this issue Mar 17, 2022 · 14 comments
Assignees
Labels
8.6-candidate discuss Issue needs discussion QA:Needs Validation Needs validation by the QA Team Team:Ecosystem Label for the Packages Ecosystem team Team:Fleet Label for the Fleet team

Comments

@akshay-saraswat
Copy link

akshay-saraswat commented Mar 17, 2022

In integrations UI, if you click on a specific agent-based integration tile to navigate to the overview page, you will see a "license" row in the details section (highlighted by the red rectangle in the screenshot below). We have been using the term "License" incorrectly in this row. It's causing a lot of confusion because the value here is Basic which is actually a subscription tier, not a license.

Acceptance Criteria:

  • Replace "License" with "Subscription" here.
  • Create a new row for a license with a new set of enum values to list ELv1, ELv2, or any other special licenses created for vector tiles or packages like ProblemChild.
  • Provide a link to license text in the license row.

image

Implementation

@akshay-saraswat akshay-saraswat added the discuss Issue needs discussion label Mar 17, 2022
@jsoriano
Copy link
Member

This will require changes in the spec and in fleet, pinging @joshdover for the changes in fleet.

Regarding the package spec, I would propose something like this:

  • license field is deprecated to avoid confusion.
  • Packages should include a LICENSE file in their root directory, with the content of their license.
  • A new condition is supported, stack.subscription, that can have the different subscriptions as values.

In fleet:

  • The "License" label is replaced by a "Subscription" label, the value would be:
    • The value in the stack.subscription condition if it is present.
    • The value in the license field otherwise (for compatibility with current packages).
  • Not sure at what points we should show the LICENSE. If we use it only on installation time, and/or after installing the package, the LICENSE could be obtained from the package itself. If we also want to show it in the integrations UI, we may need to serve it from the registry.

Alternative to the LICENSE file in the root directory of the package:

  • We limit the licenses that can be used for packages distributed through the registry.
  • Add a source object to the manifest. This object contains information about the source of the package. source.license would be one of the allowed licenses. (This would allow to have other fields as source.url with the url to the source code of the package).
  • elastic-package includes a LICENSE file in the zip files, with the text of the license indicated in source.license.
  • Fleet can show the value of source.license and can have copies of the LICENSE texts in case they need to be shown in the integrations UI for packages not installed.

@joshdover
Copy link
Contributor

Alternative to the LICENSE file in the root directory of the package:

+1 on exploring this path. I think it would make sense to have an enumerated set of allowed licenses rather than allowing arbitrary text files. This would allow us to add filtering in the future more easily and help us validate that packages are created with licenses that we've considered and are expecting. It also removes the burden from developers to find the correct license text.

@jlind23 jlind23 added the Team:Ecosystem Label for the Packages Ecosystem team label Mar 24, 2022
@jsoriano jsoriano self-assigned this Jun 15, 2022
@jsoriano
Copy link
Member

Started with the changes in the spec in #355, please have a look.

@jen-huang
Copy link
Contributor

@akshay-saraswat @jsoriano Hi, I'm reviewing the work that has been done so far here and want to clarify the acceptance criteria for license text. NOTICE.txt was introduced in #151 with the intention of it being supplemental license information. The below screenshot is how we currently display this.

How should we consolidate this with the new LICENSE.txt introduced in elastic/elastic-package#882? It is easy enough for Integrations UI to display both links, but I am wondering if it should be considered more from a package authoring perspective. At minimum, we need to clarify that LICENSE.txt is generated and NOTICE.txt is for additional information.

image

@jsoriano
Copy link
Member

jsoriano commented Jul 4, 2022

@jen-huang I understand the LICENSE.txt as the text of the license of the package itself, NOTICE.txt would be for additional considerations the distribution of the package may have, for example dependencies or included third party content that require to be mentioned in the derived work (the package) according to their licenses.

Regarding UI, I think that both LICENSE.txt and NOTICE.txt ca fit in the "License" section. The subscription (basic in this screenshot) should be moved to a new "Subscription" section.

@jen-huang
Copy link
Contributor

@jsoriano Is this ready for Kibana changes? With elastic-package tooling, can I spin up a registry that contains packages with the spec changes?

@jen-huang
Copy link
Contributor

As discussed with @jsoriano over Slack, the registry API needs to expose the new source.license field for Fleet to be able to read it (elastic/kibana#115032 would prevent the need for registry changes like this, but until that is resolved we will need to live with this).

@jsoriano
Copy link
Member

jsoriano commented Aug 2, 2022

I think most pieces are in place, and Jen is working on the implementation in Kibana. The missing piece would be to properly apply the licenses to packages. For that, package developers should at least include a LICENSE.txt file in built packages, and depending on the license, include headers in files.

Most, if not all, of the packages we develop now are in a monorepo or in a subdirectory of a repository. In these cases there is a license file at the root of the repository, that would apply to the packages, but this license is not included now.

I propose to do this:

  • In packages where a LICENSE.txt file exists, this file is copied to the built package (this is the current behaviour, this would apply to packages that live in their own repository or have their own license).
  • In packages without a LICENSE.txt file in their source directory, the following rules apply at elastic-package build time:
    • If there is a LICENSE.txt file at the root of the repository, this file is copied to the root of the built package (this would apply to packages in the integrations repository). Update: Implemented in Include repository license in package when available elastic-package#920.
    • [Alternative] We add a setting in _dev/build/build.yml indicating the location of the license file. If not set, the license in the root of the repository is used.
  • If the license requires headers in files, it is responsibility of the developer to maintain them updated, manually or using go-licenser or similar tools.

@elastic/ecosystem wdyt? This would require some changes in elastic-package.

Pinging also @andresrc and @andrewkroh for awareness and in case they have other ideas.

@mtojek
Copy link
Contributor

mtojek commented Aug 2, 2022

I like this approach, it's clean and straightforward. Alternatively, we can improve the go-licenser to detect a package, but not sure about the complexity.

@jsoriano
Copy link
Member

I guess we can close this as all tasks have been completed. Thanks @jen-huang for implementing the Fleet part!

@ghost
Copy link

ghost commented Sep 28, 2022

Hi Team

  • We have validated this feature on latest 8.5 BC1 Kibana cloud environment and found it working fine.

Build Details:
BUILD: 56595
COMMIT: 0d8de4df69f8084a94cdd9638d7de510813cb5ce

Further we have created 03 testcases for this feature after reviewing under our Fleet Test plan.

Thanks

@ghost
Copy link

ghost commented Oct 4, 2022

Hi Team,

We have executed 03 testcases for this feature under our Fleet Test run at Fleet 8.5.0-BC2 Feature test plan and found that it's working fine.

Below are the observations:

  • Passed: 02
  • Blocked: 01 (Due to Apache integration hasn't been updated in production yet to include the license field)

Build details:

Version: 8.5.0 BC2
Build: 56806
Commit: dc769f45a5a6dafb0a8c8f0c0cabcced4df45e11

Thanks!

@ghost
Copy link

ghost commented Nov 21, 2022

Hi Team,

We have updated test cases created under #298 (comment) for this feature according to latest 8.6 BC1 Kibana Cloud environment.

Build Details:

Version: 8.6.0 BC1
BUILD: 58392
COMMIT: 50a7feb0a5eb068d3acccc49c83b9ccb6db6734f

Below are the test cases.

Please let us know if we are missing anything.
Thanks

@ghost
Copy link

ghost commented Dec 6, 2022

Hi Team,

We have executed 03 testcases for this feature under our Fleet Test run at Fleet Team 8.6 New features test plan and found that it's working fine.

Build Details:

Version: 8.6.0 BC5
BUILD: 58693
COMMIT: ed40c16ce9999cc47ad55c11bb097d2e443b31a6
Artifact link: https://staging.elastic.co/8.6.0-8cf9e954/summary-8.6.0.html

Below are the test cases.

Please let us know if we are missing anything.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.6-candidate discuss Issue needs discussion QA:Needs Validation Needs validation by the QA Team Team:Ecosystem Label for the Packages Ecosystem team Team:Fleet Label for the Fleet team
Projects
None yet
Development

No branches or pull requests

6 participants