camunda-platform-helm
camunda-platform-helm copied to clipboard
camunda
[ISSUE] readme CI workflow operates on main rather than PR, sometimes causing main to not have updated readme
Describe the issue:
A recent security change we introduced was to prevent github pushes on the main branch. One of the side effects of that change is that this CI workflow that updates the README based off of our values.yaml comments is no longer capable of running because it operates on main and requires permissions to push off main.
HOWEVER, If you change the github actions workflow to operate just on the pull requests branch, this will flag an issue in our Github Security page because in the workflow, it will be reported as an insecure "git clone" because it would need to reference an actual branch instead of the commit hash.
To fix main, we will need to update the readme and then push that to main.
For future CI runs, we will either have to
- Ignore the github security notice, and allow the update-readme github actions to operate on the PR branch
- Selectively allow this CI workflow to pass on main
- Disable the update-readme workflow (or make it more manual of a step.)
Actual behavior:
CI for update readme workflow fails because main is not properly synchronized.
Expected behavior:
CI for update readme workflow should be capable of running without dependencies on the main branch.
How to reproduce:
Logs:
Environment:
Please note: Without the following info, it's hard to resolve the issue and probably it will be closed.
- Platform:
- Helm CLI version:
- Chart version:
- Values file:
The above linked PR #1498 does not fix this issue, but it does correct the readme file in main.
Daniel included the readme.update command inside the renovate post-action github actions. this will hopefully reduce the occurrences of this problem.
I'm thinking of maybe directing the CI to reference a commit hash instead of the entire PR. Currently not sure if this is possible, or would go against the security changes made.
Another solution would be to only run the readme-update workflow on the main branch.
The user could add a readme-update label on the PR.
When the PR is merged, the readme-update workflow can run on the main branch if the label was present on the PR.
This is not the best solution since the user wouldn't see what changes happened in the readme when the Ci in the PR is finished. I think this solution is similar to the second solution that Jesse mentioned above.
@drodriguez-305
How viable is it for us to disregard the security notice only for the the readme-update workflow and allow it to operate on the PR branch?
I wonder how the test-integration-template workflow is able to reference the changes in the template files within the PR, but the readm-update workflow can't reference the changes in the values.yaml.
Trying to see why that is the case.
I can also look at how bitnami uses their readme generator in their CI.
From reading this https://github.com/orgs/community/discussions/25305#discussioncomment-8256560 its quite possible we can add the security restrictions with rulesets. Then we can add the distro-ci app to bypass them. Let's discuss next week.
For now I have removed the legacy branch protections and replaced them with rulesets.
I created two rulesets. From my understanding other users were having problems when using one ruleset with the options enabled so we can try this method of separating it.
- Default branch (main)- require PR & checks (can be bypass by our Distro -CI apps)
- Default branch (main) - restrict deletions and force push
We can test it out next week.
Since I was medic last week, I didn't find time to work on this issue. I will try work on this issue this week. I am still currently medic for this week.
Had a meeting with @drodriguez-305 We saw that only the branch protection rule of not being able to push to main directly effects this workflow. We created a test pr to see if we could get the workflow to push to a new pr instead of the master branch. We were successfully able to do so but then ended up hitting a security vulnerability.
The code scanning is complaining about setting a ref when checking out the PR. I can't seem to find a workaround.
In the end we decided with @aabouzaid 's guidance that we will just add a fail message if the user did not manually run the readme generator. We decided to go down this route because there is already an existing issue that will automate the readme generator and golden files in this epic.