Kratos Charmhub review

charmhub requests review requests
1

we would like the Kratos belonging to the Identity Platform to be searchable /visible in Charmub.

Can you please help us to have it reviewed?

https://charmhub.io/kratos

Metadata links

CI Links

Documentation Links

2

Thanks @shipperizer for the review request! @dnplas can you please help with this review? You can go through and tick off the items on the checklist available here and post the result in this thread. This prior listing request could be an example of how to go about it.

1 Like
3

Definitely! On it!

4

@shipperizer, thanks for submitting the review request, some comments:

  • Kratos page - in many other charms we have reference links and some documentation or links to the documentation. Adding this information can be beneficial for users that want to integrate with the Identity solutions or want to know how to get started with the charm. I see there is information in the repo’s README.md, but it would be nice to have a more user friendly and public facing documentation, as most user’s first interactions will be with charmhub.io. Here’s a nice example.

  • Intended functionality - to be able to review this, could you please point me in the direction of testing instructions or a super simple getting started guide? What are the things I should check for to know the charm is in fact working?

  • Release automation - super tiny comment. The publish job is the automation that pushes builds and publishes a built charm(s) to a specific channel, which in turn creates a new revision; the release job on the other hand, helps you promote charms from one channel and revision to another w/o needing to re-build. This subtle distinction makes some people confuse the automation jobs, specially because “publish” and “release” are close to have the same meaning. If your team agrees, maybe you could rename “release” with “promote” instead. Not a hard requirement for approving this request, though.

  • Unit tests results - @varshigupta I will need your help with this one, while I don’t see a link to unit tests results, I’m not sure any of the repositories have this. For checking the results of the CI I usually go to the Actions section in the repo or directly check the CI run for the last commit in HEAD, but I’m not sure if we have another expectation for this. How should the Identity team resolve this item?

  • Installation test results - @varshigupta same as above.

  • Documentation for usage - the link you have provided is broken https://charmhub.io/kratos-operator

Here is the checklist:

Checked Review item Objective Review criteria
? Intended functionality Despite all the items for publication readiness, the charm must work. Charm does what it is meant to do - ideally done in a demo.
X Charmhub.io charm detail page A complete and consistent appearance of the charm is required for a quality impression of the charm collection. The overall appearance looks good, which means:
* The name complies with the naming guidelines.
* The publisher is identified.
* The links are provided.
* The documentation looks reasonable.
âś“ Source repository Generally, the source code for charms must be accessible by the community for transparency and collaboration. It is not entirely mandatory to have the charm published as OSS for review, but the repository must be accessible from the persons working on the review request.
âś“ Coding conventions The source code of the charm is accessible in the sense of approachability. Consistent source code style and formatting are also considered a sign of being committed to quality. The implemented checks for coding conventions are reasonable and implemented in the regular CI/CD implementation.
âś“ Release automation implementation An implementation for automated releasing to charmhub.io improves the ability to provide updates covering vulnerabilities quickly. Release automation for unstable channels to enable testing when new versions of the charm code or the workload become available.
âś“ Unit tests implementation In particular, for the charms review, assuring a reasonable test suite is essential to allow for automated releases in future. The unit tests show relevant coverage. It is a case-dependent review.At the time of review, the test runs successfully.
? Unit tests results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
âś“ Installation test implemented (could be part of the integration test) In particular, for the charm review, assuring a reasonable test suite is essential to allow for automated releases in future. With this test, for every build, it is ensured that the installation is successful. An implementation for checking the installation is present. The implementation should also check for successful installation as part of the automation, and the workload behaves as expected. At the time of review, the test runs successfully.
? Installation test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
âś“ Integration tests implemented In particular for the review of charms, assuring a reasonable test suite is important to allow for automated releases in future. With this test, for every build, it is ensured that the integration with other charms is successful. An implementation for testing the required integrations (if applicable) is present. The implementation should also check for successful integration as part of he automation and the workload behaves as expected.
At the time of review, the test runs successfully.
âś“ Integration test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
X Documentation for usage The documentation for using the charm should be separate from the documentation for developing or contributing to the charm. URL to this documentation is present.
âś“ Documentation for contributing The documentation for contributing to the charm should be separate from the documentation for developing or using the charm. URL to this documentation is present.
âś“ Licensing statement For the charm shared, OSS or not, the licensing terms of the charm are clarified (which also implies an identified authorship of the charm). URL to the ruling licensing statement is present.
5

hey @dnplas, thanks for the comments.

6
  • Kratos page - I see, yeah the link you are mentioning is there, and while it takes users to the Identity platform docs, those don’t just talk about Kratos specifically, they seem to define the whole solution. I’ll leave it up to the team, but I’d add charm docs and link to a page where I can learn what Kratos is and how to deploy it, this can be any of the entries in here.

EDIT: maybe you could do something like this. See the bar on the left.

  • Intended functionality - Perfect, will test it out and come back with results.
  • Tests results - for me the results from the latest commit CI are perfectly fine, but I am not sure if we are expected to provide something else. Let’s wait for @varshigupta to reply.
  • Documentation for usage: maybe you could link the Getting started guide? It will take users directly to the place they need. I understand that it doesn’t really make sense to deploy Kratos alone, so the docs for usage can be the whole bundle deployment. Could you please confirm this understanding?
7

Yes, the reason for not having individual docs for each component is that it makes no sense to deploy Kratos on its own (to be precise maybe it may make sense in some cases BUT we don’t care to cater to them).

Just to be sure I understand, can we keep the docs as they are? If there are no major issues, I would rather we keep them as they for now. (they have been reviewed by the docs team and we can always improve on them after the charms have been published).

8

@dnplas Checking the last actions run is a good approach for unit and integration tests in this case. Usually, the team provides a link to the actions run to review.

1 Like
9
  • Kratos page - Oh sorry if I was not clear, I think this page still needs some work. Since this is the landing page of the charm, you have to make it obvious where the documentation is or show detailed docs in the charm landing page. Right now the link to docs is buried in the " It is currently maintained as a part of the identity-platform" phrase. I’d add at least add another section to list all those links, for example:

Documentation

  • Link to the Getting started guide
  • Link to doc that tells me information about Kratos specifically, something similar to what you have in the charm’s repo README.
  • Link to how-to guides

Optionally, add charm docs that help you link all of those docs nicely in the left hand sidebar. All the things I have mentioned in the previous paragraph, can be added in the sidebar and you won’t have to do anything else on the main page.

I think this https://charmhub.io/ranger-k8s is a nice example of a slim yet nicely organised charm page that you can use as reference.

  • Documentation usage - I suggest you update this request and provide a working link because the one you have provided in this request does not work. If you are going to link charmhub.io/kratos, then that site MUST have usage instructions.

I have updated the checklist, this would be v2.

Checked Review item Objective Review criteria
âś“ Intended functionality Despite all the items for publication readiness, the charm must work. Charm does what it is meant to do - ideally done in a demo.
X Charmhub.io charm detail page A complete and consistent appearance of the charm is required for a quality impression of the charm collection. The overall appearance looks good, which means:
* The name complies with the naming guidelines.
* The publisher is identified.
* The links are provided.
* The documentation looks reasonable.
âś“ Source repository Generally, the source code for charms must be accessible by the community for transparency and collaboration. It is not entirely mandatory to have the charm published as OSS for review, but the repository must be accessible from the persons working on the review request.
âś“ Coding conventions The source code of the charm is accessible in the sense of approachability. Consistent source code style and formatting are also considered a sign of being committed to quality. The implemented checks for coding conventions are reasonable and implemented in the regular CI/CD implementation.
âś“ Release automation implementation An implementation for automated releasing to charmhub.io improves the ability to provide updates covering vulnerabilities quickly. Release automation for unstable channels to enable testing when new versions of the charm code or the workload become available.
âś“ Unit tests implementation In particular, for the charms review, assuring a reasonable test suite is essential to allow for automated releases in future. The unit tests show relevant coverage. It is a case-dependent review.At the time of review, the test runs successfully.
âś“ Unit tests results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
âś“ Installation test implemented (could be part of the integration test) In particular, for the charm review, assuring a reasonable test suite is essential to allow for automated releases in future. With this test, for every build, it is ensured that the installation is successful. An implementation for checking the installation is present. The implementation should also check for successful installation as part of the automation, and the workload behaves as expected. At the time of review, the test runs successfully.
âś“ Installation test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
âś“ Integration tests implemented In particular for the review of charms, assuring a reasonable test suite is important to allow for automated releases in future. With this test, for every build, it is ensured that the integration with other charms is successful. An implementation for testing the required integrations (if applicable) is present. The implementation should also check for successful integration as part of he automation and the workload behaves as expected.
At the time of review, the test runs successfully.
âś“ Integration test results Availability of test results is mandatory for a working collaborative project. URL to test results from CI/CD automation.
X Documentation for usage The documentation for using the charm should be separate from the documentation for developing or contributing to the charm. URL to this documentation is present.
âś“ Documentation for contributing The documentation for contributing to the charm should be separate from the documentation for developing or using the charm. URL to this documentation is present.
âś“ Licensing statement For the charm shared, OSS or not, the licensing terms of the charm are clarified (which also implies an identified authorship of the charm). URL to the ruling licensing statement is present.