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

http_server: serve license files #76

Merged
merged 8 commits into from
Sep 10, 2024
Merged

Conversation

hnez
Copy link
Member

@hnez hnez commented Aug 5, 2024

Some open source license require shipping the license text along with the built software.
To comply with this requirement we let yocto generate a directory with license texts inside of it and expose it via the web interface.

This PR is developed in tandem with linux-automation/meta-lxatac#163, which installs the license files to the image in the first place.

This is what this will look like to the user:

Legal Information Page

A new entry "Legal Information" was added to the web interface. Clicking on it reveals a list of installed packages.


License files

The rightmost column in the table contains links that lead to the license text provided by the package.
Clicking on it opens the plain text file in the browser.

@hnez
Copy link
Member Author

hnez commented Aug 5, 2024

I've assigned this to @jluebbe first to decide if this is what we want.
Deciding if this is implemented in a way we want is a separate task and can be done afterwards.

@hnez
Copy link
Member Author

hnez commented Aug 6, 2024

A stated in linux-automation/meta-lxatac#163 we also want to expose the license manifest file, because it includes software version information.
This will likely not need any modifications here, just a quick check if the file displays correctly and that the tacd does not stumble over it being a symlink (if we implement it that way).

@jluebbe jluebbe assigned hnez and unassigned jluebbe Aug 22, 2024
@hnez
Copy link
Member Author

hnez commented Aug 23, 2024

This will likely not need any modifications here, just a quick check if the file displays correctly and that the tacd does not stumble over it being a symlink (if we implement it that way).

Not quite how it turned out. Symlinking the manifest file would have required some hacks on the meta-lxatac side, so instead I decided to "do it properly" and serve the manifest file and the license text files at different locations and use React code to tie them together.
The result can be seen in the screenshots above.
The manifest file is parsed in the web browser and is displayed as a table containing the package name, version and license. The license link is clickable and leads to the license text.

@hnez hnez assigned KarlK90 and unassigned hnez Aug 26, 2024
@hnez
Copy link
Member Author

hnez commented Aug 30, 2024

Hi @KarlK90, I know that web development is your secret passion.
Could you have a look at this PR from a Rust and Typescript perspective?

@KarlK90
Copy link
Member

KarlK90 commented Sep 2, 2024

Hi @KarlK90, I know that web development is your secret passion. Could you have a look at this PR from a Rust and Typescript perspective?

I'm sorry to inform you that this is not true, but I'll try my best. 😉

Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hnez LGTM, but if possible I would like to see a unit-test for the license file.

Comment on lines +38 to +41
// PACKAGE NAME: tacd
// PACKAGE VERSION: 1.0.0
// RECIPE NAME: tacd
// LICENSE: GPL-2.0-or-later
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How stable is the format generated by poky/meta/classes-recipe/license_image.bbclass? How hard would it be to have some kind of unit-test against a recent license file?

id: "license",
header: "License",
cell: (p) => (
<Link href={"/docs/legal/files/" + p.recipe_name}>{p.license}</Link>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first thought that we could just set the mime type in the link on the front end, but we also generate a file listing. So you're approach is probably a good abstraction.

@hnez
Copy link
Member Author

hnez commented Sep 9, 2024

@hnez LGTM, but if possible I would like to see a unit-test for the license file.

Now that I have re-read this message I am not too sure if I actually implemented what you have asked for.
I've just pushed a few commits just now that add a web interface unit test for parsing the included demo_mode manifest file.

But that does not check against a recent actual manifest file. That would be a bit more involved, because the manifest is generated in meta-lxatac, not in the tacd.

We could add a test there, to check if the format did not change. But I think just keeping an eye on the yocto release notes for changes to the file format may be a better option. I would assume that a (breaking) change to the manifest file format would be treated as noteworthy enough to be listed in the release notes.

@SmithChart
Copy link
Member

We could also add an integration test into labgrid-lxatac to check if the functionality on the actual device does still work. Or is the manifest processed during buildtime?

hnez added 6 commits September 9, 2024 15:09
The `serve_dir` function already returns a suitable Future,
so there is no need to wrap it inside an extra async block.

Signed-off-by: Leonard Göhrs <[email protected]>
This will come in handy for license files,
which do not have an extension that can be used to guess the mime type,
but which are known to be plain text.

Signed-off-by: Leonard Göhrs <[email protected]>
Some open source license require shipping the license text along with
the built software.
To comply with this requirement we let yocto generate a directory with
license texts inside of it and expose it via the web interface.

Signed-off-by: Leonard Göhrs <[email protected]>
The create-react-app project has already prepared everything for us so we
can run unit tests on our code base.
All that is left to set up are some tweaks so that jest runs babel on the
cloudscape dependency, because otherwise the tests will trip over
javascript features that are just too new.

Signed-off-by: Leonard Göhrs <[email protected]>
The test uses the manifest file used in the demo mode as a reference.
The file is very minimal, as it only has two entries, but the format is
very regular, so how many entries it has should not make much of a
difference.

Signed-off-by: Leonard Göhrs <[email protected]>
hnez added 2 commits September 9, 2024 15:09
The test does not check the rendering output, only that it does in fact
render without crashing.
An actual test would require a deeper understanding of the jsdom DOM
mocking than I currently have.
Maybe we can add it in the future.

Signed-off-by: Leonard Göhrs <[email protected]>
Until now we did not have any tests to run, but that has changed now.

Run the new tests in a CI job.

Signed-off-by: Leonard Göhrs <[email protected]>
@hnez
Copy link
Member Author

hnez commented Sep 9, 2024

The manifest is only processed at runtime, so that would be an option.
The actual parsing does however take place in the browser and not on the TAC, making a full integration test complicated.

@SmithChart
Copy link
Member

The manifest is only processed at runtime, so that would be an option. The actual parsing does however take place in the browser and not on the TAC, making a full integration test complicated.

Indeed. I'll add it our test backlog anyway. So if we really want to we can add Selenium or what ever is the browser-test-engine-of-the-day to our dependencies.

@hnez
Copy link
Member Author

hnez commented Sep 10, 2024

Hi, it would be great to get this merged, since it is the only open PR before we can start preparing a new meta-lxatac release.

I am not quite sure if there is anything I need to do.

Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I have re-read this message I am not too sure if I actually implemented what you have asked for.
I've just pushed a few commits just now that add a web interface unit test for parsing the included demo_mode manifest file.

@hnez The tests are a good start and go in the right direction. My intention was to have some kind of integration test with the real license file so that the functionality doesn't silently break.

We could add a test there, to check if the format did not change. But I think just keeping an eye on the yocto release notes for changes to the file format may be a better option. I would assume that a (breaking) change to the manifest file format would be treated as noteworthy enough to be listed in the release notes.

Famous last words 😉. Realistically this dependency will be forgotten over time by anyone involved. I'm not a fan of such solutions as it increases the mental load on what to look out for in a new release.

Indeed. I'll add it our test backlog anyway. So if we really want to we can add Selenium or what ever is the browser-test-engine-of-the-day to our dependencies.

@SmithChart Playwright seems to be the goto solution and is already used in our company with success.

All in all I'm fine with merging this PR with the hope that we'll gain front end testing in the future to catch regressions.

@hnez hnez merged commit a519eb3 into linux-automation:main Sep 10, 2024
15 checks passed
@hnez hnez deleted the serve-licenses branch September 10, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants