Closed carslen closed 5 months ago
Hi @tom-rm-meyer-ISST the repo is ready for review Hi @RoKrish14 could you please take over the security checks?
First of all: Sorry for waiting that long. Currently everything is a rush :( But I also used this QGate check to cross check some things I need to update for our repo, too :)
QGate checklist lacks TRG 2.06, which is successfully fulfilled.
TRG 7.06 (frontend about license) is not applicable thus ticked.
Dear co-pilot @carslen, I would like to have your opinion on the following topics (also feel free to check the findings). @evegufy feel free to also discuss - maybe we should schedule a meeting :)
From my understanding this is fulfilled (already ticked 3.02) as this is only applicable to the Postgres dependency, which comes with it by default.
The values.yaml by default does not make the PVC / StatefulSet configurable, which is OK from my point of view as I don't think I've currently seen it anywhere (and we also didn't do it).
Do you agree, that it's fine or do you think it's not following the TRG?
TRG 7.04 clarifies third party content while TRG 7.01 explains the Notice file.
This repository currently contains the Dashtool as a jar at ./scripts/download
. I would expect to have a NOTICE.md
beside it or better even remove it and add a step to download it to the respective point in workflows / development documentation. I'm also not fully sure about the license here (we're apache 2.0, dash is EPL-2.0). What do you think.
Also following the EF handbook, I noticed that the following excerpt of the NOTICE.md
that should be used. It mentions the dash tool usage, but we don't have it mentioned in the TRG example. Any idea because I think no repo has it mentioned at all (we until now don't have it):
# Notices for Dash, Tools for Committers
This content is produced and maintained by the Eclipse Dash, Tools for
Committers project.
* Project home: https://projects.eclipse.org/projects/technology.dash
The digest stated in the should example is only a SHOULD and not a MUST, isn't it? If it's a must I would need to uncheck TRG 5.05.
Note: As a few findings are pretty small, I fixed them already in a PR.
app
with 1654:1654
since version 8 (that you use). I would go for it :)Hi @tom-rm-meyer-ISST Thank you for performing the thorough review and especially for opening #145!
Regarding the findings: I've already commented some of the issues and I think only https://github.com/eclipse-tractusx/policy-hub/issues/142 is remaining, that one I'll implement today or tomorrow.
Regarding the Dash Tool: I'll just change to downloading at every run, I had that anyway on my TODO list but due to time constraints, I wasn't able to come around to.
Regarding the recommendations: thank you for the input and we might look into those in the future. Just letting you know that it's possible to link to files with spaces in the file name by using %20
: https://github.com/eclipse-tractusx/policy-hub/blob/main/docs/technical-documentation/architecture/Development%20Concept.md
Hi @tom-rm-meyer-ISST the repo is ready for review Hi @RoKrish14 could you please take over the security checks?
Hi @RoKrish14 could you please take over the security checks and leave a comment on the following issue as well https://github.com/eclipse-tractusx/sig-release/issues/672? Thank you!
Hi @tom-rm-meyer-ISST Thank you for performing the thorough review and especially for opening #145! Regarding the findings: I've already commented some of the issues and I think only #142 is remaining, that one I'll implement today or tomorrow. Regarding the Dash Tool: I'll just change to downloading at every run, I had that anyway on my TODO list but due to time constraints, I wasn't able to come around to. Regarding the recommendations: thank you for the input and we might look into those in the future. Just letting you know that it's possible to link to files with spaces in the file name by using
%20
: https://github.com/eclipse-tractusx/policy-hub/blob/main/docs/technical-documentation/architecture/Development%20Concept.md
Thanks for the fast answers and clarifications! Also thanks for pointing out the encoding of spaces. Didn't think about that! :)
Hi @tom-rm-meyer-ISST the repo is ready for review Hi @RoKrish14 could you please take over the security checks?
Hi @RoKrish14 could you please take over the security checks and leave a comment on the following issue as well eclipse-tractusx/sig-release#672? Thank you!
Done 👍
Hi @tom-rm-meyer-ISST the repo is ready for review Hi @RoKrish14 could you please take over the security checks?
Also done 👍
Finally walked through closed issues and PRs. Everything fine from my perspective. Some issues were false positive, recommendations and discussion points are already resolved. Thanks for collaborating; it was fun!
Finally walked through closed issues and PRs. Everything fine from my perspective. Some issues were false positive, recommendations and discussion points are already resolved. Thanks for collaborating; it was fun!
Thank you for the great review! The final version is released now: https://github.com/eclipse-tractusx/policy-hub/releases/tag/policy-hub-1.0.0
QG checks
Please keep this issue open until QG is concluded and will be managed by the Issue Creator! We will inform you about finding and proposals in separated issues, this issue here is for the Overview of the Checks!
Please keep this issue open until QG is concluded!
Product Owner: @jjeroch Dev SPOC: @evegufy Helm Chart Version: 1.0.0 App Version: 1.0.0
Release Managemnet Reference Issue: eclipse-tractusx/sig-release#672
Check of Tractus-X Release Guidelines
TRG 1 Documentation
README.md
INSTALL.md
or inREADME.md
CHANGELOG.md
TRG 2 Git
main
.tractusx
metafile in a proper formatTRG 3 Kubernetes
TRG 4 Container
USER
command and Non Root ContainerDockerHub
, removeGHCR
referencesDockerHub
has all necessary informationTRG 5 Helm
/charts
directory and correct structureTRG 6 Released Helm Chart
TRG 7 Open Source Governance
TRG 8 Security
Hints
Information Sharing