Generator: Improve CI/CD

[RobertDiebels]

Suggestions to improve CI/CD

Would like to request the following improvements to the CI/CD and building of the project:

• ~Don't use .sh-files.~
• ~Don't use Makefile.~
• ~Add static code analysis tooling.~
• Split CI/CD jobs.
• Split CI/CD stages.

Reasoning

Here's my reasoning behind each suggestion.

Don't use .sh-files

I noticed that a lot of OSS projects which target different operating systems actually cannot be build on all of the OS they're targeting. Most of the time this is due to using Linux-specific commands throughout their build. Using .sh files will increase the chance that, over time, Linux-specific commands are introduced since they're originally used for Unix shell commands. Hence my suggestion to not use them.

Don't use Makefile

A Makefile is intended to be read by make a Unix-specific building tool. Support for make on Windows machines is subpar. Relying on it to build cross-platform tooling would mean you can't run a build on a Windows machine and then test it. Instead you'd be required to either install things like CygWin or in the worst case have both a Unix machine and a Windows machine. That's why I'd suggest not using it.

Add static code analysis tooling

I've recently had to do a deep dive into the code of a generator for a different specification. I won't name the generator but the code was really unclear. Files with over 1000 lines, if nesting over 4 statements deep, unclear method naming etc. This makes the project harder to maintain, decreases overall quality and increases the time to build new features. To avoid this from happening here I'd like to suggest adding static code analysis tooling to the CI/CD process.

Split CI/CD jobs

The generator currently has 1 job that builds, tests and deploys the generator. Splitting that job across multiple jobs will make the CI/CD process clearer and adds a fail-safe in case one of the commands fails.

Split CI/CD stages

The generator currently has 1 job in 1 stage. Adding stages should help to clarify which stage the build is in and if it failed in which stage it failed. This is useful when you end up having to create a multitude of jobs.

Solutions

When you suggest improvements you usually also have a suggestion on how to accomplish them. So here they are.

Don't use .sh-files. / Don't use Makefile

Simply use the language you're using in the project's build-scripts. This ensures that everyone working on the project knows how the build works. If they don't, at the very least they won't have to learn a lot of OS-specific commands. Since the scripts are written in the language of the project with which they should already be familiar.

Add static code analysis tooling

I know of either BetterCodeHub (free for OSS) or SonarCloud (free for OSS). There might be others as well so feel free to suggest some.

Split CI/CD jobs / Split CI/CD stages

This depends on Travis but the documentation is pretty straight forward about it.
Jobs -> listing
Stages -> defining

[derberg]

@RobertDiebels great summary! I'm very curious about the subject related to bash and make.

Simply use the language you're using in the project's build-scripts.

This sounds pretty reasonable. I have some questions though:

• Do you have some examples of the projects that do it this way?
• We have some projects written in Go, this would mean that we would have to kind of do a double work, same scripts write in 2 different languages, doesn't feel nice, or am I missing something?

Another solution maybe would be to use docker to run all those scripts and make targets. Docker will provide an isolated environment that will work the same on any platform. What say you?

[RobertDiebels]

Do you have some examples of the projects that do it this way?

Unfortunately I only know of projects that did not do it this way. When Hyperledger Fabric was on version 1.2 I tried building their code on my Windows machine for my graduate project. It was impossible. They used .sh everywhere and some of the libraries they used depended on Linux as the host-OS.
Both Golang and Javascript can easily run on a variety of operating systems. So to me creating builds that depend on the specifics of a particular OS doesn't make sense. Unless you can always control the OS on which the scripts are run.

I realize that the only .sh file in the repo at this point is meant for releases. Which should be fine because you want control over the machines that create a release. Usually though this tends to set a precedent for other build-scripts to also be placed inside .sh files.

[...] this would mean that we would have to kind of do a double work, same scripts write in 2 different languages, doesn't feel nice, or am I missing something?

I think that would only be a problem if you want to ensure each project has the exact same build-structure. What I've learned is that each project will diverge at some point regardless. You're right though, the exact same scripts in different languages does seem odd.

[derberg]

Both Golang and Javascript can easily run on a variety of operating systems. So to me creating builds that depend on the specifics of a particular OS doesn't make sense. Unless you can always control the OS on which the scripts are run.

You can control the OS if you run the scripts inside a docker container and I think that in case of our projects this is much simpler than setting up Node.js script or Go executable to execute few Docker commands inside. Then it is just a matter of proper instructions.

Or even better, maybe look closer into https://github.com/nektos/act and profit from the fact that we use GitHub Actions and we can run them locally with act which sounds nifty

What I've learned is that each project will diverge at some point regardless. You're right though, the exact same scripts in different languages does seem odd.

I think we are not a large project with a lot of repositories, so for us it should be crucial to be consistent across all repos to copy between them the best setup and make it easier to setup new projects.

In my opinion, the only argument to run away from .sh scripts is when your have large project with CI scripts that are long and scary and it is about time to introduce tests for the scripts.

[RobertDiebels]

You can control the OS if you run the scripts inside a docker container and I think that in case of our projects this is much simpler than setting up Node.js script or Go executable to execute few Docker commands inside.

True. However, I don't like introducing dependencies for local builds when they're not necessary. The reason you'd need Docker is because you want to keep using .sh files and possibly Unix-specific commands. When you should be able to just do npm run build and be done with it regardless of the operating system you're on.

Or even better, maybe look closer into https://github.com/nektos/act [...]

I looked through act's documentation but they make no reference of Windows as a platform they support. In fact, their documentation proves my point. To build their code or run unit-tests [ building from source ] I need to use make. Which is not available on Windows.

I think we are not a large project with a lot of repositories, so for us it should be crucial to be consistent across all repos to copy between them the best setup and make it easier to setup new projects.

Excluding the AsyncAPI website and specification related repositories, I count at least 8 repositories which use either Golang or Javascript. Copying build-scripts around for those repositories and potentially opening them up for local build issues is not a good practice.

In my opinion, the only argument to run away from .sh scripts is when your have large project with CI scripts that are long and scary and it is about time to introduce tests for the scripts.

I strongly disagree with that statement. The reason you'd want to avoid scripts that are not intended to be cross-platform is because you want to maximize the amount of potential contributors and decrease the amount of time necessary to setup a local development environment. Especially if you're working on a project that is intended to be deployed to a variety of operating systems.

In this case the Makefile adds a dependency to make which does not work well on Windows. Unless you're using a local VM or something like CygWIN. To me, setting up a VM or CygWin just to build a docker image and create a release seems like a waste of time.
Currently the release.sh file already contains a Linux-only command in the form of $PWD which means that it only works on Windows if I use PowerShell. Even-tough it contains just 5 lines of commands. This means that I cannot easily create releases with the current setup on my local machine.

Again, in the case of releases I can get that you'd use Unix-specific commands. However, I would place the commands inside a CI-file such as the main.yml file you introduced for GitHub workflows. In those scripts you have control over the system which is building a release and you're not implying that anyone can create a release by just running the .sh or Makefile.

Adding the commands to the CI-file would remove the need for make on my local machine and implicitly the need for the Makefile and the .sh files in the project.

[derberg]

I looked through act's documentation but they make no reference of Windows as a platform they support.

It is written in Go, prividing executable for windows should not be a problem. I see they provide some assets in releases https://github.com/nektos/act/releases

Copying build-scripts around for those repositories and potentially opening them up for local build issues is not a good practice.

I could argue here a bit, but I won't 😄 basically it depends 😉

The reason you'd want to avoid scripts that are not intended to be cross-platform is because you want to maximize the amount of potential contributors and decrease the amount of time necessary to setup a local development environment

Another area where we can argue a bit but I do not think it makes sense :) I agree we need to make projects as easy to start with as possible. I do not believe though that such an improvement is a meaningful enabler. As you noticed at the begging, it is not easy to find a good role model projects. Anyway, let's park this part.

---

In general, awesome we are having this exchange of thoughts as now I understand more what you are suggesting, at the beginning, I somehow assumed you want a huge change, but what you would like to achieve in fact is as simple as that:

Remove Makefile and release.sh and instead move it's targets to package.json scripts to be able to do npm run docker-build, npm run docker-tag (it can accept an argument, with tag name, that we would grab automatically directly inside GitHub Action, but you could also pass it manually locally if you want), npm run docker-release that triggers previous scripts and pushed to docker hub

Please write that I'm getting it right now 😃

[derberg]

@RobertDiebels We agreed on the open meeting to go forward with this one. Any suggestions on how we proceed with this one? closing it and create separate issues for execution that link to this one as a reference to the initial discussion?

[RobertDiebels]

@derberg Regarding your earlier comment . My intention was not to move the release.sh commands to package.json but to the main.yml file for GitHub actions. Releases should be created by the CI system automatically. If you'd want to build a container locally for testing purposes you'd run a docker command in the terminal of your OS.

As for moving forward with this issue:

• My suggestion would be to just use this issue because the change is not that big. Two files will be removed ('Makefile', release.sh), main.yml file will be edited.
• The static code analysis should also be placed inside the GitHub actions main.yml.
• If we're doing this I suggest we also remove the .travis.yml file and move its jobs over to the GitHub actions main.yml fole. Since having two CI-systems side-by-side seems like overkill.

[derberg]

I totally agree with the steps forward you suggested.
I would only suggest encapsulating docker commands, that are useful for local development, inside the npm scripts, it would be: https://github.com/asyncapi/generator/blob/master/Makefile#L3. The rest that contributor will not find useful for local development and they are strictly related to release, they go to main.yml directly, as you mentioned.

[RobertDiebels]

Seems good to me. Would you like to pick this up or should I?

[derberg]

Feel free to pick it up. At the moment I'm on https://github.com/asyncapi/generator/issues/182 and https://github.com/asyncapi/asyncapi/issues/312.

[derberg]

hint, for GH Action where we need to pick latest tag, I would suggest to use GH Action like https://github.com/WyriHaximus/github-action-get-previous-tag instead of some custom solution we have atm

[derberg]

and having eslint PR verification in place would be awesome :) https://github.com/reviewdog/action-eslint

[RobertDiebels]

@derberg The get previous tag seems great! Will take a look at it asap.
ESlint has a a lot of overlap with other static analysis tooling. I'd have to look into exactly how much overlap there is. Otherwise there would be duplicated warning messages.

[derberg]

we use ESLint for autoformatting so we still need it, unless there is a other way

did you start working on it already? I could take the sh/make file topic as I anyway start working on https://github.com/asyncapi/generator/issues/225 and could handle the scripts refactor

[RobertDiebels]

@derberg ESLint formatting is 100% ok. Most static analysis tooling only indicate QA related warnings/errors. Possibly there will be some overlap in messages such as: Unused variable X, remove it. etc. I'd need to check the current ESlint settings and see if there is overlap there.

I have not started on this issue yet. There is another issue [ https://github.com/asyncapi/generator/issues/201 ] that is assigned to me and I feel I should continue on that one first so get to know the inner workings of the generator a bit better.
If you want to start on the .sh/Makefile then by all means :D !

[derberg]

@RobertDiebels hey man, I'm done with makefile/release.sh/jobs organization

Have a look https://github.com/asyncapi/generator/pull/242

I can now jump into static code analysis if you didn't start it yet

[RobertDiebels]

@derberg Just looked through https://github.com/asyncapi/generator/pull/242 . Had one comment.
Feel free to jump in. I'm swamped with work and Uni again. Should ease off next month.

[derberg]

@RobertDiebels thanks for the clarification. I'll jump to it then, thanks

I also answered your comment, I added docker-build script to package.json as per my previous comment https://github.com/asyncapi/generator/issues/220#issuecomment-589976549

[derberg]

Regarding static code analysis. After some research I say we go with Sonar Cloud that is:

• we can integrate existing ESLint rules with it if we want https://blog.sonarsource.com/import-issues-of-your-favorite-linters-in-sonarcloud.

• to detect some SonarJS errors before pushing we just add a plugin to eslint:

  npm install eslint-plugin-sonarjs@latest --save-dev
  
  //and below to eslintrc
  plugins: 
    - sonarjs
  
  extends: 
    - plugin:sonarjs/recommended
  

  It only triggers few more errors, which is not bad after all.

• it does security validation
• we get a very nice summary from the bot on a PR level with check that later can be made mandatory -> https://github.com/asyncapi/generator/pull/256
• current errors that are not urgent - we could create issues from them, they would be great onboarding issues for contributors. Small refactoring of code helps to learn it

I now enabled it for generator for testing sample https://sonarcloud.io/dashboard?id=asyncapi_generator

---

@RobertDiebels @fmvilas @jonaslagoni guys, for me the case is simple, so simple that I've already created a PR in generator :D https://github.com/asyncapi/generator/pull/256

What do you think? should I check something more? do we need more?

[fmvilas]

I think this is fine. I don't know many alternatives, to be honest. I know there are a lot but it's been loooong time I don't use these tools.

[RobertDiebels]

@derberg #256 is only meant for eslint integration right? Because ideally Sonar would run before a release as well in Github Actions.

EDIT: Sonar also has an IDE-plugin for most popular IDE's called SonarLint. It helps you pick up these issues before a commit. I believe you can also attach it to a project's quality gate. Which ensures you're always using the quality settings of the project.

[derberg]

I think IDE usage is specific to a developer, I use VS Code and I know there are 3 Sonar-like plugins. #256 is for ESLint integration with Sonar rules, so we can combine both and someone that already uses ESLint with IDE will have all work out of the box.

SonarCloud is already integrated with Generator for testing purposes on PRs https://github.com/asyncapi/generator/pull/256#issuecomment-601261884

I suggest we do not add it to GitHub Action for release yet, let us give it like 3 or so releases to see if we are happy with how it works. There are some issues we need to resolve first, ESLint is throwing some already

---

what you mean by quality gate? like having a status badge in repository readme or?

[derberg]

btw, SonarCloud GH action is already there https://github.com/marketplace/actions/sonarcloud-scan ❤️

[RobertDiebels]

@derberg

[...] so we can combine both and someone that already uses ESLint with IDE will have all work out of the box.

Gotcha.

SonarCloud is already integrated with Generator for testing purposes on PRs [...]

I am aware of that.

I suggest we do not add it to GitHub Action for release yet, let us give it like 3 or so releases to see if we are happy with how it works. [...]

I don't really see a reason to wait +/- 3 releases. Especially if you're already introducing the bot. Adding a GH action won't infringe much on the release-process imho.

what you mean by quality gate? [...]

Sonar has a variety of topics for which it does static analysis, maintainability, security, etc. It has a set of rules it verifies per topic which it calls measures. The outcomes of those measures determine a project's quality grading.

A quality gate is any combination of the grade-requirements that a project should meet. Depending on the settings of the quality gate you can set a build or PR to fail based on a set of grade requirements.
For instance:
Maintainability > B, the maintainability grading must to be higher than a B grading.
Reliability > C, the reliability grading must to be higher than a C grading.

There can be many combinations. This is just sample. Based on a quality gate you can then fail or allow a build, PR, etc.

According to Sonar: "A Quality Gate is a set of measure-based Boolean conditions. It helps you know immediately whether your project is production-ready. If your current status is not Passed, you'll see which measures caused the problem and the values required to pass."

[derberg]

I don't really see a reason to wait +/- 3 releases. Especially if you're already introducing the bot. Adding a GH action won't infringe much on the release-process imho.

Cause bot is now neutral, not blocking PRs, as we do not yet know how helpful/anoying it will be with code changes. So we should not have it in release workflow if we do not have it mandatory on PR

---

Thanks for explanation of quality gates, got it

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions :heart:

[derberg]

This one is still valid, we are pretty happy with SonarCube and also its eslint extension so next to follow is rollout to other repositories

[RobertDiebels]

Still following along, just not capable of adding more work onto the pile. Should ease off within a month or 2.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions :heart:

[derberg]

SonarCloud is now configured to watch all current and futuere AsyncAPI repositories.
I will add separate issues per repository about:

• eslint plugin for nodejs plugins
• fixing issues found by sonarcloud
• making sonarcloud mandatory check
• making sonarcloud check run on release