-
Notifications
You must be signed in to change notification settings - Fork 102
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
Support for YAML and Helm chart static analysis #582
base: master
Are you sure you want to change the base?
Conversation
YamlParser now parses the .yaml files in the given project. It reads the yaml files and persists the data into db tables
YamlParser now parses the .yaml files in the given project. It reads the yaml files and persists the data into db tables
YamlParser now parses the .yaml files in the given project. It reads the yaml files and persists the data into db tables
Full content of the yaml file is now persisted into the db tables
Now yaml files can be classified as kubernetes config, CI, Helm Chart or Other file.
…menuitem for displaying YamlFileInfo
…mportant keys from the File
…rent types of Kubernetes diagrams.
(Just a side note: if we are embedding business logic into this module that is specific to specific YAML schemae (I see Helm mentioned in the original post) then maybe calling the plugin "YAML Parser" is a bit too generic.) |
The plugin became more and more specific to Helm and Kubernetes from the initial point of plain YAML parsing. I plan to rename it to "Microservice plugin" or something like that. |
…lue analyzer class.
I added the most important modifications to the PR. It is now in a state where parsing is correct and thorough enough to provide useful functionality. I removed the WIP flag, as the PR is ready for review. |
# Conflicts: # .github/workflows/ci.yml
Resolved merge conflicts in 71047a8. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changeset generally meets the coding guidelines, but multiple naming conventions (helm, yaml and microservice) creates confusion.
When testing on the gitlab Helm chart, I received a segmentation fault, therefore I could not test the functionality.
Also, the update for the tarball creation (new dependency) is missing from the PR.
@@ -0,0 +1,161 @@ | |||
# Helm chart static analysis plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this file be linked somewhere in the upper documentation files? Currently it is quite hard to find.
@@ -31,6 +31,7 @@ function (lang, Deferred, model) { | |||
InfoTree : 6, /** Info tree item (eg. CppInforTree)**/ | |||
FileManagerContextMenu : 7, /** File manager context menu (eg. Diagram directory)**/ | |||
InfoPage : 8, /** Info page (eg. Credit, User Guide) **/ | |||
MicroserviceContextMenu: 9 /** Microservice navigator context menu **/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, helm
vs. yaml
vs. microservice
. Three different naming for the same thing. Confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I would leave it like this. The primary purpose of the Helm chart analysis is to identify microservices and their connections, so I think this naming is correct here. I'm unifying it in other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand this plugin is capable for Kubernetes microservice analysis based on Helm charts. Naming it simply microservice analysis in general is a bit like naming the "C++ plugin" as "Programming language plugin", as that would be a much wider concept.
There are other microservice orchestration frameworks beside Kubernetes, like Docker Swarm, Helios, Amazon ECS or Mesos, just to name a few. This plugin does not deal with those systems, instead it is fundamentally coupled with Helm charts for Kubernetes. Thus, in my opinion, the naming should use the helm
keyword mostly and also towards the end user it should be named something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the context menu would be Microservice -> Helm charts -> Subitems, that would also be correct of course.
if (m != std::string::npos) | ||
{ | ||
amount_.erase(m, 2); | ||
LOG(info) << std::stof(amount_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and other log commands produces some strange and verbose output, e.g.:
2023-08-10 15:44:30 [INFO] 50Gi
2023-08-10 15:44:30 [INFO] 50
2023-08-10 15:44:30 [INFO] 256
2023-08-10 15:44:30 [INFO] 8Gi
2023-08-10 15:44:30 [INFO] 8
2023-08-10 15:44:30 [INFO] 8Gi
2023-08-10 15:44:30 [INFO] 8
There should be more context and the messages either debug or trace level.
This is a new language plugin to extend CC with. Ericsson products have been transforming their projects from monolithic to service oriented software architecture so they are now built up of microservices. Since projects are very large and sprawling, there is a wide demand in Ericsson for a static analysis tool that will help understanding the network of microservices within a project.
This plugin consists of
I'll keep expanding this plugin with the huge amount of feature requests that I received from Ericsson, but I think that the plugin can be officially added to CC at this point with the current feature set.
The basis of this work was done by @rizwankadhar as his BSc thesis. Thanks Rizwan for your work!